Skip to content

Commit

Permalink
Change IAM binding to be authoritative (#1116)
Browse files Browse the repository at this point in the history 
Merged PR #1116.
  • Loading branch information
@chrisst @modular-magician
chrisst authored and modular-magician committed Jan 4, 2019
1 parent 1b78e77 commit e339329
Show file tree
Hide file tree
Showing 15 changed files with 174 additions and 123 deletions.
2 changes: 1 addition & 1 deletion build/terraform
Submodule terraform updated from 505cb4 to 43d40f
2 changes: 1 addition & 1 deletion build/terraform-beta
Submodule terraform-beta updated from 3889c3 to be11e1
44 changes: 4 additions & 40 deletions third_party/terraform/resources/resource_iam_binding.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand All @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
140 changes: 81 additions & 59 deletions third_party/terraform/tests/resource_google_project_iam_policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"},
},
},
},
Expand Down Expand Up @@ -416,7 +438,7 @@ data "google_iam_policy" "expanded" {
"user:paddy@carvers.co",
]
}
binding {
role = "roles/viewer"
members = [
Expand Down
19 changes: 19 additions & 0 deletions third_party/terraform/utils/iam.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions third_party/terraform/website/docs/d/google_iam_policy.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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, alice@gmail.com or joe@example.com.
* **user:{emailid}**: An email address that represents a specific Google account. For example, alice@gmail.com.
* **serviceAccount:{emailid}**: An email address that represents a service account. For example, my-other-app@appspot.gserviceaccount.com.
* **group:{emailid}**: An email address that represents a Google group. For example, admins@example.com.
* **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.
Expand Down
8 changes: 6 additions & 2 deletions third_party/terraform/website/docs/r/google_billing_account_iam_binding.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -23,7 +27,7 @@ resource "google_billing_account_iam_binding" "binding" {
role = "roles/billing.viewer"
members = [
"user:jane@example.com",
"user:alice@gmail.com",
]
}
```
Expand All @@ -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

Expand Down
6 changes: 3 additions & 3 deletions third_party/terraform/website/docs/r/google_billing_account_iam_member.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
```

Expand All @@ -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
Expand Down
Loading

0 comments on commit e339329

Please sign in to comment.