Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support organization iam conditions #4749

Merged
merged 3 commits into from
Apr 30, 2021
Merged

Support organization iam conditions #4749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f0bd28
Split out organization iam tests
melinath Apr 28, 2021
60f9449
Added failing tests for organization iam binding and member with cond…
melinath Apr 29, 2021
537e86c
Made organization iam policy requests include requested policy version
melinath Apr 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions mmv1/third_party/terraform/resources/resource_iam_binding.go.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"regexp"
"strings"

"github.com/davecgh/go-spew/spew"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"google.golang.org/api/cloudresourcemanager/v1"
Expand Down Expand Up @@ -130,8 +131,8 @@ func resourceIamBindingRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Rea
if err != nil {
return handleNotFoundError(err, d, fmt.Sprintf("Resource %q with IAM Binding (Role %q)", updater.DescribeResource(), eBinding.Role))
}
log.Printf("[DEBUG] Retrieved policy for %s: %+v", updater.DescribeResource(), p)
log.Printf("[DEBUG] Looking for binding with role %q and condition %+v", eBinding.Role, eCondition)
log.Print(spew.Sprintf("[DEBUG] Retrieved policy for %s: %#v", updater.DescribeResource(), p))
c2thorn marked this conversation as resolved.
Show resolved Hide resolved
log.Printf("[DEBUG] Looking for binding with role %q and condition %#v", eBinding.Role, eCondition)

var binding *cloudresourcemanager.Binding
for _, b := range p.Bindings {
Expand All @@ -143,7 +144,7 @@ func resourceIamBindingRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Rea

if binding == nil {
log.Printf("[WARNING] Binding for role %q not found, assuming it has no members. If you expected existing members bound for this role, make sure your role is correctly formatted.", eBinding.Role)
log.Printf("[DEBUG] Binding for role %q and condition %+v not found in policy for %s, assuming it has no members.", eBinding.Role, eCondition, updater.DescribeResource())
log.Printf("[DEBUG] Binding for role %q and condition %#v not found in policy for %s, assuming it has no members.", eBinding.Role, eCondition, updater.DescribeResource())
if err := d.Set("role", eBinding.Role); err != nil {
return fmt.Errorf("Error setting role: %s", err)
}
Expand Down
9 changes: 5 additions & 4 deletions mmv1/third_party/terraform/resources/resource_iam_member.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"regexp"
"strings"

"github.com/davecgh/go-spew/spew"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"google.golang.org/api/cloudresourcemanager/v1"
Expand Down Expand Up @@ -222,8 +223,8 @@ func resourceIamMemberRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Read
if err != nil {
return handleNotFoundError(err, d, fmt.Sprintf("Resource %q with IAM Member: Role %q Member %q", updater.DescribeResource(), eMember.Role, eMember.Members[0]))
}
log.Printf("[DEBUG]: Retrieved policy for %s: %+v\n", updater.DescribeResource(), p)
log.Printf("[DEBUG]: Looking for binding with role %q and condition %+v", eMember.Role, eCondition)
log.Print(spew.Sprintf("[DEBUG]: Retrieved policy for %s: %#v\n", updater.DescribeResource(), p))
log.Printf("[DEBUG]: Looking for binding with role %q and condition %#v", eMember.Role, eCondition)

var binding *cloudresourcemanager.Binding
for _, b := range p.Bindings {
Expand All @@ -234,7 +235,7 @@ func resourceIamMemberRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Read
}

if binding == nil {
log.Printf("[DEBUG]: Binding for role %q with condition %+v does not exist in policy of %s, removing member %q from state.", eMember.Role, eCondition, updater.DescribeResource(), eMember.Members[0])
log.Printf("[DEBUG]: Binding for role %q with condition %#v does not exist in policy of %s, removing member %q from state.", eMember.Role, eCondition, updater.DescribeResource(), eMember.Members[0])
d.SetId("")
return nil
}
Expand All @@ -248,7 +249,7 @@ func resourceIamMemberRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Read
}

if member == "" {
log.Printf("[DEBUG]: Member %q for binding for role %q with condition %+v does not exist in policy of %s, removing from state.", eMember.Members[0], eMember.Role, eCondition, updater.DescribeResource())
log.Printf("[DEBUG]: Member %q for binding for role %q with condition %#v does not exist in policy of %s, removing from state.", eMember.Members[0], eMember.Role, eCondition, updater.DescribeResource())
d.SetId("")
return nil
}
Expand Down
164 changes: 156 additions & 8 deletions mmv1/third_party/terraform/tests/resource_google_organization_iam_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,32 @@ import (
// behavior however induces flakiness in our acceptance tests, hence the need for running them
// serially.
// Policies are *not tested*, because testing them will ruin changes made to the test org.
func TestAccOrganizationIam(t *testing.T) {
func TestAccOrganizationIamMembersAndBindings(t *testing.T) {
if os.Getenv("TF_RUN_ORG_IAM") != "true" {
t.Skip("Environment variable TF_RUN_ORG_IAM is not set, skipping.")
}

t.Parallel()

testCases := map[string]func(t *testing.T){
"bindingBasic": testAccOrganizationIamBinding_basic,
"bindingCondition": testAccOrganizationIamBinding_condition,
"memberBasic": testAccOrganizationIamMember_basic,
"memberCondition": testAccOrganizationIamMember_condition,
}

for name, tc := range testCases {
// shadow the tc variable into scope so that when
// the loop continues, if t.Run hasn't executed tc(t)
// yet, we don't have a race condition
// see https://github.com/golang/go/wiki/CommonMistakes#using-goroutines-on-loop-iterator-variables
tc := tc
t.Run(name, func(t *testing.T) {
tc(t)
})
}
}

func testAccOrganizationIamBinding_basic(t *testing.T) {
org := getTestOrgFromEnv(t)
account := fmt.Sprintf("tf-test-%d", randInt(t))
roleId := "tfIamTest" + randString(t, 10)
Expand All @@ -34,7 +53,7 @@ func TestAccOrganizationIam(t *testing.T) {
Steps: []resource.TestStep{
{
// Test Iam Binding creation
Config: testAccOrganizationIamBinding_basic(account, roleId, org),
Config: testAccOrganizationIamBinding_basicConfig(account, roleId, org),
Check: testAccCheckGoogleOrganizationIamBindingExists(t, "foo", "test-role", []string{
fmt.Sprintf("serviceAccount:%s@%s.iam.gserviceaccount.com", account, getTestProjectFromEnv()),
}),
Expand All @@ -59,9 +78,46 @@ func TestAccOrganizationIam(t *testing.T) {
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func testAccOrganizationIamBinding_condition(t *testing.T) {
org := getTestOrgFromEnv(t)
account := fmt.Sprintf("tf-test-%d", randInt(t))
roleId := "tfIamTest" + randString(t, 10)
conditionTitle := "expires_after_2019_12_31"
vcrTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
// Test Iam Binding creation
Config: testAccOrganizationIamBinding_conditionConfig(account, roleId, org, conditionTitle),
Check: testAccCheckGoogleOrganizationIamBindingExists(t, "foo", "test-role", []string{
fmt.Sprintf("serviceAccount:%s@%s.iam.gserviceaccount.com", account, getTestProjectFromEnv()),
}),
},
{
ResourceName: "google_organization_iam_binding.foo",
ImportStateId: fmt.Sprintf("%s organizations/%s/roles/%s %s", org, org, roleId, conditionTitle),
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func testAccOrganizationIamMember_basic(t *testing.T) {
org := getTestOrgFromEnv(t)
account := fmt.Sprintf("tf-test-%d", randInt(t))
vcrTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
// Test Iam Member creation (no update for member, no need to test)
Config: testAccOrganizationIamMember_basic(account, org),
Config: testAccOrganizationIamMember_basicConfig(account, org),
Check: testAccCheckGoogleOrganizationIamMemberExists(t, "foo", "roles/browser",
fmt.Sprintf("serviceAccount:%s@%s.iam.gserviceaccount.com", account, getTestProjectFromEnv()),
),
Expand All @@ -76,6 +132,37 @@ func TestAccOrganizationIam(t *testing.T) {
})
}

func testAccOrganizationIamMember_condition(t *testing.T) {
org := getTestOrgFromEnv(t)
account := fmt.Sprintf("tf-test-%d", randInt(t))
conditionTitle := "expires_after_2019_12_31"
vcrTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
// Test Iam Member creation (no update for member, no need to test)
Config: testAccOrganizationIamMember_conditionConfig(account, org, conditionTitle),
Check: testAccCheckGoogleOrganizationIamMemberExists(t, "foo", "roles/browser",
fmt.Sprintf("serviceAccount:%s@%s.iam.gserviceaccount.com", account, getTestProjectFromEnv()),
),
},
{
ResourceName: "google_organization_iam_member.foo",
ImportStateId: fmt.Sprintf(
"%s roles/browser serviceAccount:%s@%s.iam.gserviceaccount.com %s",
org,
account,
getTestProjectFromEnv(),
conditionTitle,
),
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func testAccCheckGoogleOrganizationIamBindingExists(t *testing.T, bindingResourceName, roleResourceName string, members []string) resource.TestCheckFunc {
return func(s *terraform.State) error {
bindingRs, ok := s.RootModule().Resources["google_organization_iam_binding."+bindingResourceName]
Expand All @@ -89,7 +176,14 @@ func testAccCheckGoogleOrganizationIamBindingExists(t *testing.T, bindingResourc
}

config := googleProviderConfig(t)
p, err := config.NewResourceManagerClient(config.userAgent).Organizations.GetIamPolicy("organizations/"+bindingRs.Primary.Attributes["org_id"], &cloudresourcemanager.GetIamPolicyRequest{}).Do()
p, err := config.NewResourceManagerClient(config.userAgent).Organizations.GetIamPolicy(
"organizations/"+bindingRs.Primary.Attributes["org_id"],
&cloudresourcemanager.GetIamPolicyRequest{
Options: &cloudresourcemanager.GetPolicyOptions{
RequestedPolicyVersion: iamPolicyVersion,
},
},
).Do()
if err != nil {
return err
}
Expand Down Expand Up @@ -119,7 +213,14 @@ func testAccCheckGoogleOrganizationIamMemberExists(t *testing.T, n, role, member
}

config := googleProviderConfig(t)
p, err := config.NewResourceManagerClient(config.userAgent).Organizations.GetIamPolicy("organizations/"+rs.Primary.Attributes["org_id"], &cloudresourcemanager.GetIamPolicyRequest{}).Do()
p, err := config.NewResourceManagerClient(config.userAgent).Organizations.GetIamPolicy(
"organizations/"+rs.Primary.Attributes["org_id"],
&cloudresourcemanager.GetIamPolicyRequest{
Options: &cloudresourcemanager.GetPolicyOptions{
RequestedPolicyVersion: iamPolicyVersion,
},
},
).Do()
if err != nil {
return err
}
Expand All @@ -142,7 +243,7 @@ func testAccCheckGoogleOrganizationIamMemberExists(t *testing.T, n, role, member

// We are using a custom role since iam_binding is authoritative on the member list and
// we want to avoid removing members from an existing role to prevent unwanted side effects.
func testAccOrganizationIamBinding_basic(account, role, org string) string {
func testAccOrganizationIamBinding_basicConfig(account, role, org string) string {
return fmt.Sprintf(`
resource "google_service_account" "test-account" {
account_id = "%s"
Expand Down Expand Up @@ -194,7 +295,34 @@ resource "google_organization_iam_binding" "foo" {
`, account, role, org, account, org)
}

func testAccOrganizationIamMember_basic(account, org string) string {
func testAccOrganizationIamBinding_conditionConfig(account, role, org, conditionTitle string) string {
return fmt.Sprintf(`
resource "google_service_account" "test-account" {
account_id = "%s"
display_name = "Organization Iam Testing Account"
}
resource "google_organization_iam_custom_role" "test-role" {
role_id = "%s"
org_id = "%s"
title = "Iam Testing Role"
permissions = ["genomics.datasets.get"]
}
resource "google_organization_iam_binding" "foo" {
org_id = "%s"
role = google_organization_iam_custom_role.test-role.id
members = ["serviceAccount:${google_service_account.test-account.email}"]
condition {
title = "%s"
description = "Expiring at midnight of 2019-12-31"
expression = "request.time < timestamp(\"2020-01-01T00:00:00Z\")"
}
}
`, account, role, org, org, conditionTitle)
}

func testAccOrganizationIamMember_basicConfig(account, org string) string {
return fmt.Sprintf(`
resource "google_service_account" "test-account" {
account_id = "%s"
Expand All @@ -208,3 +336,23 @@ resource "google_organization_iam_member" "foo" {
}
`, account, org)
}

func testAccOrganizationIamMember_conditionConfig(account, org, conditionTitle string) string {
return fmt.Sprintf(`
resource "google_service_account" "test-account" {
account_id = "%s"
display_name = "Organization Iam Testing Account"
}
resource "google_organization_iam_member" "foo" {
org_id = "%s"
role = "roles/browser"
member = "serviceAccount:${google_service_account.test-account.email}"
condition {
title = "%s"
description = "Expiring at midnight of 2019-12-31"
expression = "request.time < timestamp(\"2020-01-01T00:00:00Z\")"
}
}
`, account, org, conditionTitle)
}
3 changes: 2 additions & 1 deletion mmv1/third_party/terraform/utils/iam.go.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strings"
"time"

"github.com/davecgh/go-spew/spew"
"github.com/hashicorp/errwrap"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"google.golang.org/api/cloudresourcemanager/v1"
Expand Down Expand Up @@ -69,7 +70,7 @@ func iamPolicyReadWithRetry(updater ResourceIamUpdater) (*cloudresourcemanager.P
if err != nil {
return nil, err
}
log.Printf("[DEBUG] Retrieved policy for %s: %+v\n", updater.DescribeResource(), policy)
log.Print(spew.Sprintf("[DEBUG] Retrieved policy for %s: %#v\n", updater.DescribeResource(), policy))
return policy, nil
}

Expand Down
9 changes: 8 additions & 1 deletion mmv1/third_party/terraform/utils/iam_organization.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,14 @@ func (u *OrganizationIamUpdater) GetResourceIamPolicy() (*cloudresourcemanager.P
return nil, err
}

p, err := u.Config.NewResourceManagerClient(userAgent).Organizations.GetIamPolicy("organizations/"+u.resourceId, &cloudresourcemanager.GetIamPolicyRequest{}).Do()
p, err := u.Config.NewResourceManagerClient(userAgent).Organizations.GetIamPolicy(
"organizations/"+u.resourceId,
&cloudresourcemanager.GetIamPolicyRequest{
Options: &cloudresourcemanager.GetPolicyOptions{
RequestedPolicyVersion: iamPolicyVersion,
},
},
).Do()
if err != nil {
return nil, errwrap.Wrapf(fmt.Sprintf("Error retrieving IAM policy for %s: {{err}}", u.DescribeResource()), err)
}
Expand Down