Skip to content

Commit

Permalink
Fix IAM policy behavior regression for empty members (hashicorp#1092)
Browse files Browse the repository at this point in the history
Signed-off-by: Modular Magician <[email protected]>
  • Loading branch information
modular-magician authored and emilymye committed Aug 27, 2019
1 parent 8f0d10a commit 4447526
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 3 deletions.
12 changes: 9 additions & 3 deletions google-beta/iam.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
18 changes: 18 additions & 0 deletions google-beta/iam_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
38 changes: 38 additions & 0 deletions google-beta/resource_google_project_iam_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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" {
Expand Down

0 comments on commit 4447526

Please sign in to comment.