From 013fa464535e9c6650202737b69aaef8c67f624b Mon Sep 17 00:00:00 2001 From: Chase Vedder Date: Tue, 9 Apr 2024 17:18:32 +0000 Subject: [PATCH 1/2] Fix bug that prevented setting 'create_ignore_already_exists' on existing resources --- .../resource_google_service_account.go | 11 +--- .../resource_google_service_account_test.go | 63 +++++++++++++++++++ 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/mmv1/third_party/terraform/services/resourcemanager/resource_google_service_account.go b/mmv1/third_party/terraform/services/resourcemanager/resource_google_service_account.go index 12385029257e..99a61fa63512 100644 --- a/mmv1/third_party/terraform/services/resourcemanager/resource_google_service_account.go +++ b/mmv1/third_party/terraform/services/resourcemanager/resource_google_service_account.go @@ -266,21 +266,16 @@ func resourceGoogleServiceAccountUpdate(d *schema.ResourceData, meta interface{} if err != nil { return err } - - if len(updateMask) == 0 { - return nil - } - } else if d.HasChange("disabled") && d.Get("disabled").(bool) { _, err = config.NewIamClient(userAgent).Projects.ServiceAccounts.Disable(d.Id(), &iam.DisableServiceAccountRequest{}).Do() if err != nil { return err } + } - if len(updateMask) == 0 { - return nil - } + if len(updateMask) == 0 { + return nil } _, err = config.NewIamClient(userAgent).Projects.ServiceAccounts.Patch(d.Id(), diff --git a/mmv1/third_party/terraform/services/resourcemanager/resource_google_service_account_test.go b/mmv1/third_party/terraform/services/resourcemanager/resource_google_service_account_test.go index fabfc3605030..06aff6f58f9d 100644 --- a/mmv1/third_party/terraform/services/resourcemanager/resource_google_service_account_test.go +++ b/mmv1/third_party/terraform/services/resourcemanager/resource_google_service_account_test.go @@ -2,6 +2,7 @@ package resourcemanager_test import ( "fmt" + "strings" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -129,6 +130,57 @@ func TestAccServiceAccount_createIgnoreAlreadyExists(t *testing.T) { }) } +// Test setting create_ignore_already_exists on an existing resource +func TestAccServiceAccount_existingResourceCreateIgnoreAlreadyExists(t *testing.T) { + t.Parallel() + + project := envvar.GetTestProjectFromEnv() + + serviceAccount := map[string]interface{}{ + "account_id": "a" + acctest.RandString(t, 10), + "display_name": "Terraform Test", + "description": "test description", + } + + serviceAccountConfig := newServiceAccountResource(serviceAccount) + + serviceAccount["create_ignore_already_exists"] = true + serviceAccountConfigIgnoreAlreadyExists := newServiceAccountResource(serviceAccount) + + expectedEmail := fmt.Sprintf("%s@%s.iam.gserviceaccount.com", serviceAccount["account_id"], project) + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + // The first step creates a new resource with create_ignore_already_exists omitted + { + Config: serviceAccountConfig, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "google_service_account.acceptance", "project", project), + resource.TestCheckResourceAttr( + "google_service_account.acceptance", "member", "serviceAccount:"+expectedEmail), + ), + }, + { + ResourceName: "google_service_account.acceptance", + ImportStateId: fmt.Sprintf("projects/%s/serviceAccounts/%s", project, expectedEmail), + ImportState: true, + ImportStateVerify: true, + }, + // The second step updates the resource to have create_ignore_already_exists=true + { + Config: serviceAccountConfigIgnoreAlreadyExists, + Check: resource.ComposeTestCheckFunc(resource.TestCheckResourceAttr( + "google_service_account.acceptance", "project", project), + resource.TestCheckResourceAttr( + "google_service_account.acceptance", "member", "serviceAccount:"+expectedEmail), + ), + }, + }, + }) +} + func TestAccServiceAccount_Disabled(t *testing.T) { t.Parallel() @@ -197,6 +249,17 @@ func testAccStoreServiceAccountUniqueId(uniqueId *string) resource.TestCheckFunc } } +func newServiceAccountResource(options map[string]interface{}) string { + var sb strings.Builder + sb.WriteString(`resource "google_service_account" "acceptance" {`) + sb.WriteString("\n") + for key, value := range options { + sb.WriteString(fmt.Sprintf("\t%v\t= \"%v\"\n", key, value)) + } + sb.WriteString(`}`) + return sb.String() +} + func testAccServiceAccountBasic(account, name, desc string) string { return fmt.Sprintf(` resource "google_service_account" "acceptance" { From 1307264b6058ff6477cccbca9eb749d54106516a Mon Sep 17 00:00:00 2001 From: Chase Vedder Date: Mon, 15 Apr 2024 19:09:03 +0000 Subject: [PATCH 2/2] Update tests to be in line with existing standards --- .../resource_google_service_account_test.go | 58 ++++++++----------- 1 file changed, 25 insertions(+), 33 deletions(-) diff --git a/mmv1/third_party/terraform/services/resourcemanager/resource_google_service_account_test.go b/mmv1/third_party/terraform/services/resourcemanager/resource_google_service_account_test.go index 06aff6f58f9d..12fa5d1d6bb9 100644 --- a/mmv1/third_party/terraform/services/resourcemanager/resource_google_service_account_test.go +++ b/mmv1/third_party/terraform/services/resourcemanager/resource_google_service_account_test.go @@ -2,7 +2,6 @@ package resourcemanager_test import ( "fmt" - "strings" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -120,7 +119,7 @@ func TestAccServiceAccount_createIgnoreAlreadyExists(t *testing.T) { }, // The second step creates a new resource that duplicates with the existing service account. { - Config: testAccServiceAccountCreateIgnoreAlreadyExists(accountId, displayName, desc), + Config: testAccServiceAccountDuplicateIgnoreAlreadyExists(accountId, displayName, desc), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr( "google_service_account.duplicate", "member", "serviceAccount:"+expectedEmail), @@ -135,26 +134,18 @@ func TestAccServiceAccount_existingResourceCreateIgnoreAlreadyExists(t *testing. t.Parallel() project := envvar.GetTestProjectFromEnv() + accountId := "a" + acctest.RandString(t, 10) + displayName := "Terraform Test" + desc := "test description" - serviceAccount := map[string]interface{}{ - "account_id": "a" + acctest.RandString(t, 10), - "display_name": "Terraform Test", - "description": "test description", - } - - serviceAccountConfig := newServiceAccountResource(serviceAccount) - - serviceAccount["create_ignore_already_exists"] = true - serviceAccountConfigIgnoreAlreadyExists := newServiceAccountResource(serviceAccount) - - expectedEmail := fmt.Sprintf("%s@%s.iam.gserviceaccount.com", serviceAccount["account_id"], project) + expectedEmail := fmt.Sprintf("%s@%s.iam.gserviceaccount.com", accountId, project) acctest.VcrTest(t, resource.TestCase{ PreCheck: func() { acctest.AccTestPreCheck(t) }, ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), Steps: []resource.TestStep{ - // The first step creates a new resource with create_ignore_already_exists omitted + // The first step creates a new resource with create_ignore_already_exists=false { - Config: serviceAccountConfig, + Config: testAccServiceAccountCreateIgnoreAlreadyExists(accountId, displayName, desc, false), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr( "google_service_account.acceptance", "project", project), @@ -163,14 +154,15 @@ func TestAccServiceAccount_existingResourceCreateIgnoreAlreadyExists(t *testing. ), }, { - ResourceName: "google_service_account.acceptance", - ImportStateId: fmt.Sprintf("projects/%s/serviceAccounts/%s", project, expectedEmail), - ImportState: true, - ImportStateVerify: true, + ResourceName: "google_service_account.acceptance", + ImportStateId: fmt.Sprintf("projects/%s/serviceAccounts/%s", project, expectedEmail), + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"create_ignore_already_exists"}, // Import leaves this field out when false }, // The second step updates the resource to have create_ignore_already_exists=true { - Config: serviceAccountConfigIgnoreAlreadyExists, + Config: testAccServiceAccountCreateIgnoreAlreadyExists(accountId, displayName, desc, true), Check: resource.ComposeTestCheckFunc(resource.TestCheckResourceAttr( "google_service_account.acceptance", "project", project), resource.TestCheckResourceAttr( @@ -249,17 +241,6 @@ func testAccStoreServiceAccountUniqueId(uniqueId *string) resource.TestCheckFunc } } -func newServiceAccountResource(options map[string]interface{}) string { - var sb strings.Builder - sb.WriteString(`resource "google_service_account" "acceptance" {`) - sb.WriteString("\n") - for key, value := range options { - sb.WriteString(fmt.Sprintf("\t%v\t= \"%v\"\n", key, value)) - } - sb.WriteString(`}`) - return sb.String() -} - func testAccServiceAccountBasic(account, name, desc string) string { return fmt.Sprintf(` resource "google_service_account" "acceptance" { @@ -270,7 +251,18 @@ resource "google_service_account" "acceptance" { `, account, name, desc) } -func testAccServiceAccountCreateIgnoreAlreadyExists(account, name, desc string) string { +func testAccServiceAccountCreateIgnoreAlreadyExists(account, name, desc string, ignore_already_exists bool) string { + return fmt.Sprintf(` +resource "google_service_account" "acceptance" { + account_id = "%v" + display_name = "%v" + description = "%v" + create_ignore_already_exists = %t +} +`, account, name, desc, ignore_already_exists) +} + +func testAccServiceAccountDuplicateIgnoreAlreadyExists(account, name, desc string) string { return fmt.Sprintf(` resource "google_service_account" "acceptance" { account_id = "%v"