Skip to content

Commit

Permalink
Fix cloud_identity_group_membership to properly handle 403 responses (#…
Browse files Browse the repository at this point in the history
…7089) (#5103)

* Fix cloud_identity_group_membership to properly handle 403 responses when membership does not exist

* Fix service account creation errors to cause entire test to fail immediately

* Skip VCR for this test to avoid managing service accounts out-of-band

Signed-off-by: Modular Magician <magic-modules@google.com>

Signed-off-by: Modular Magician <magic-modules@google.com>
  • Loading branch information
modular-magician authored Jan 13, 2023
1 parent 25fe035 commit 6a0d5e5
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 1 deletion.
3 changes: 3 additions & 0 deletions .changelog/7089.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:none

```
27 changes: 27 additions & 0 deletions google-beta/cloud_identity_group_membership_utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package google

import (
"log"
"strings"

"github.com/hashicorp/errwrap"
"google.golang.org/api/googleapi"
)

func transformCloudIdentityGroupMembershipReadError(err error) error {
if gErr, ok := errwrap.GetType(err, &googleapi.Error{}).(*googleapi.Error); ok {
if gErr.Code == 403 && strings.Contains(gErr.Message, "(or it may not exist)") {
// This error occurs when either the group membership does not exist, or permission is denied. It is
// deliberately ambiguous so that existence information is not revealed to the caller. However, for
// the Read function, we can only assume that the membership does not exist, and proceed with attempting
// other operations. Since handleNotFoundError(...) expects an error code of 404 when a resource does not
// exist, to get the desired behavior, we modify the error code to be 404.
gErr.Code = 404
}

log.Printf("[DEBUG] Transformed CloudIdentityGroupMembership error")
return gErr
}

return err
}
2 changes: 1 addition & 1 deletion google-beta/resource_cloud_identity_group_membership.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func resourceCloudIdentityGroupMembershipRead(d *schema.ResourceData, meta inter

res, err := sendRequest(config, "GET", billingProject, url, userAgent, nil)
if err != nil {
return handleNotFoundError(err, d, fmt.Sprintf("CloudIdentityGroupMembership %q", d.Id()))
return handleNotFoundError(transformCloudIdentityGroupMembershipReadError(err), d, fmt.Sprintf("CloudIdentityGroupMembership %q", d.Id()))
}

if err := d.Set("name", flattenCloudIdentityGroupMembershipName(res["name"], d, config)); err != nil {
Expand Down
85 changes: 85 additions & 0 deletions google-beta/resource_cloud_identity_group_membership_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"google.golang.org/api/iam/v1"
)

func TestAccCloudIdentityGroupMembership_update(t *testing.T) {
Expand Down Expand Up @@ -175,6 +176,90 @@ resource "google_cloud_identity_group_membership" "basic" {
`, context)
}

func TestAccCloudIdentityGroupMembership_membershipDoesNotExist(t *testing.T) {
// Skip VCR because the service account needs to be created/deleted out of
// band, and so those calls aren't recorded
skipIfVcr(t)
t.Parallel()

context := map[string]interface{}{
"org_domain": getTestOrgDomainFromEnv(t),
"cust_id": getTestCustIdFromEnv(t),
"random_suffix": randString(t, 10),
}

saId := "tf-test-sa-" + randString(t, 10)
project := getTestProjectFromEnv()
config := BootstrapConfig(t)

r := &iam.CreateServiceAccountRequest{
AccountId: saId,
ServiceAccount: &iam.ServiceAccount{},
}

sa, err := config.NewIamClient(config.userAgent).Projects.ServiceAccounts.Create("projects/"+project, r).Do()
if err != nil {
t.Fatalf("Error creating service account: %s", err)
}

context["member_id"] = sa.Email

vcrTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckCloudIdentityGroupMembershipDestroyProducer(t),
Steps: []resource.TestStep{
{
Config: testAccCloudIdentityGroupMembership_dne(context),
},
{
PreConfig: func() {
config := googleProviderConfig(t)

_, err := config.NewIamClient(config.userAgent).Projects.ServiceAccounts.Delete(sa.Name).Do()
if err != nil {
t.Errorf("cannot delete service account %s: %v", sa.Name, err)
return
}
},
Config: testAccCloudIdentityGroupMembership_dne(context),
PlanOnly: true,
ExpectNonEmptyPlan: true,
},
},
})
}

func testAccCloudIdentityGroupMembership_dne(context map[string]interface{}) string {
return Nprintf(`
resource "google_cloud_identity_group" "group" {
display_name = "tf-test-my-identity-group-%{random_suffix}"
parent = "customers/%{cust_id}"
group_key {
id = "tf-test-my-identity-group-%{random_suffix}@%{org_domain}"
}
labels = {
"cloudidentity.googleapis.com/groups.discussion_forum" = ""
}
}
resource "google_cloud_identity_group_membership" "basic" {
group = google_cloud_identity_group.group.id
preferred_member_key {
id = "%{member_id}"
}
roles {
name = "MEMBER"
}
}
`, context)
}

func TestAccCloudIdentityGroupMembership_cloudIdentityGroupMembershipWithMemberKey(t *testing.T) {
t.Parallel()

Expand Down

0 comments on commit 6a0d5e5

Please sign in to comment.