Skip to content

Commit

Permalink
Fix range issue over gcp iam policy bindings.
Browse files Browse the repository at this point in the history
- When a binding got removed the first time, it would actually shorten
the loop. The next binding to be removed would cause a slice bounds out
of range issue. Instead of using golang's range, used a traditional for
loop to maintain the integrity of the "length" of the loop.

Fixes #73
  • Loading branch information
Genevieve Lesperance committed Jan 24, 2019
1 parent 1ae139f commit 4bbb9ed
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 18 deletions.
17 changes: 4 additions & 13 deletions gcp/iam/service_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,32 +55,23 @@ func (s ServiceAccount) removeBindings() error {
return fmt.Errorf("Get Project IAM Policy: %s", err)
}

toRemove := []binding{}
for j := len(p.Bindings) - 1; j >= 0; j-- {
b := p.Bindings[j]

for j, b := range p.Bindings {
for i, m := range b.Members {
if strings.Contains(m, s.email) {
toRemove = append(toRemove, binding{
ServiceAccount: s.email,
Member: m,
Role: b.Role,
})

// Remove this member from the binding
b.Members = append(b.Members[:i], b.Members[i+1:]...)
}
}

if len(b.Members) == 0 {
// If there are no more members for the role, remove the whole binding
p.Bindings = append(p.Bindings[:j], p.Bindings[j+1:]...)
} else {
p.Bindings[j] = b
}
}

for _, binding := range toRemove {
s.logger.Printf("gcloud iam service-accounts remove-iam-policy-binding %s --member %s --role %s\n", binding.ServiceAccount, binding.Member, binding.Role)
}

_, err = s.client.SetProjectIamPolicy(p)
if err != nil {
return fmt.Errorf("Set Project IAM Policy: %s", err)
Expand Down
17 changes: 12 additions & 5 deletions gcp/iam/service_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,6 @@ var _ = Describe("ServiceAccount", func() {
err := serviceAccount.Delete()
Expect(err).NotTo(HaveOccurred())

Expect(logger.PrintfCall.Receives.Message).To(Equal("gcloud iam service-accounts remove-iam-policy-binding %s --member %s --role %s\n"))
Expect(logger.PrintfCall.Receives.Arguments[0]).To(Equal("[email protected]"))
Expect(logger.PrintfCall.Receives.Arguments[1]).To(Equal("serviceAccount:[email protected]"))
Expect(logger.PrintfCall.Receives.Arguments[2]).To(Equal("roles/some-role"))

Expect(client.SetProjectIamPolicyCall.CallCount).To(Equal(1))
Expect(client.SetProjectIamPolicyCall.Receives.Input).To(Equal(updatedPolicy))
})
Expand Down Expand Up @@ -107,6 +102,14 @@ var _ = Describe("ServiceAccount", func() {
Members: []string{"user:apple", "serviceAccount:[email protected]"},
Role: "roles/some-other-role",
},
{
Members: []string{"serviceAccount:[email protected]"},
Role: "roles/some-third-role",
},
{
Members: []string{"serviceAccount:[email protected]"},
Role: "roles/some-fourth-role",
},
},
}
updatedPolicy = &gcpcrm.Policy{
Expand All @@ -115,6 +118,10 @@ var _ = Describe("ServiceAccount", func() {
Members: []string{"user:apple"},
Role: "roles/some-other-role",
},
{
Members: []string{"serviceAccount:[email protected]"},
Role: "roles/some-third-role",
},
},
}
})
Expand Down

0 comments on commit 4bbb9ed

Please sign in to comment.