diff --git a/google/resource_logging_billing_account_exclusion_test.go b/google/resource_logging_billing_account_exclusion_test.go index 39c6357e704..b4a795f5d86 100644 --- a/google/resource_logging_billing_account_exclusion_test.go +++ b/google/resource_logging_billing_account_exclusion_test.go @@ -7,29 +7,42 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" - "google.golang.org/api/logging/v2" ) -func TestAccLoggingBillingAccountExclusion_basic(t *testing.T) { +// Logging exclusions don't always work when making parallel requests, so run tests serially +func TestAccLoggingBillingAccountExclusion(t *testing.T) { t.Parallel() + testCases := map[string]func(t *testing.T){ + "basic": testAccLoggingBillingAccountExclusion_basic, + "update": testAccLoggingBillingAccountExclusion_update, + "multiple": testAccLoggingBillingAccountExclusion_multiple, + } + + 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 testAccLoggingBillingAccountExclusion_basic(t *testing.T) { billingAccount := getTestBillingAccountFromEnv(t) exclusionName := "tf-test-exclusion-" + acctest.RandString(10) description := "Description " + acctest.RandString(10) - var exclusion logging.LogExclusion - resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckLoggingBillingAccountExclusionDestroy, Steps: []resource.TestStep{ { - Config: testAccLoggingBillingAccountExclusion_basic(exclusionName, description, billingAccount), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingBillingAccountExclusionExists("google_logging_billing_account_exclusion.basic", &exclusion), - testAccCheckLoggingBillingAccountExclusion(&exclusion, "google_logging_billing_account_exclusion.basic"), - ), + Config: testAccLoggingBillingAccountExclusion_basicCfg(exclusionName, description, billingAccount), }, { ResourceName: "google_logging_billing_account_exclusion.basic", @@ -40,34 +53,27 @@ func TestAccLoggingBillingAccountExclusion_basic(t *testing.T) { }) } -func TestAccLoggingBillingAccountExclusion_update(t *testing.T) { - t.Parallel() - +func testAccLoggingBillingAccountExclusion_update(t *testing.T) { billingAccount := getTestBillingAccountFromEnv(t) exclusionName := "tf-test-exclusion-" + acctest.RandString(10) descriptionBefore := "Basic BillingAccount Logging Exclusion" + acctest.RandString(10) descriptionAfter := "Updated Basic BillingAccount Logging Exclusion" + acctest.RandString(10) - var exclusionBefore, exclusionAfter logging.LogExclusion - resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckLoggingBillingAccountExclusionDestroy, Steps: []resource.TestStep{ { - Config: testAccLoggingBillingAccountExclusion_basic(exclusionName, descriptionBefore, billingAccount), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingBillingAccountExclusionExists("google_logging_billing_account_exclusion.basic", &exclusionBefore), - testAccCheckLoggingBillingAccountExclusion(&exclusionBefore, "google_logging_billing_account_exclusion.basic"), - ), + Config: testAccLoggingBillingAccountExclusion_basicCfg(exclusionName, descriptionBefore, billingAccount), }, { - Config: testAccLoggingBillingAccountExclusion_basic(exclusionName, descriptionAfter, billingAccount), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingBillingAccountExclusionExists("google_logging_billing_account_exclusion.basic", &exclusionAfter), - testAccCheckLoggingBillingAccountExclusion(&exclusionAfter, "google_logging_billing_account_exclusion.basic"), - ), + ResourceName: "google_logging_billing_account_exclusion.basic", + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccLoggingBillingAccountExclusion_basicCfg(exclusionName, descriptionAfter, billingAccount), }, { ResourceName: "google_logging_billing_account_exclusion.basic", @@ -76,19 +82,36 @@ func TestAccLoggingBillingAccountExclusion_update(t *testing.T) { }, }, }) +} - // Description should have changed, but Filter and Disabled should be the same - if exclusionBefore.Description == exclusionAfter.Description { - t.Errorf("Expected Description to change, but it didn't: Description = %#v", exclusionBefore.Description) - } - if exclusionBefore.Filter != exclusionAfter.Filter { - t.Errorf("Expected Filter to be the same, but it differs: before = %#v, after = %#v", - exclusionBefore.Filter, exclusionAfter.Filter) - } - if exclusionBefore.Disabled != exclusionAfter.Disabled { - t.Errorf("Expected Disabled to be the same, but it differs: before = %#v, after = %#v", - exclusionBefore.Disabled, exclusionAfter.Disabled) - } +func testAccLoggingBillingAccountExclusion_multiple(t *testing.T) { + billingAccount := getTestBillingAccountFromEnv(t) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckLoggingBillingAccountExclusionDestroy, + Steps: []resource.TestStep{ + { + Config: testAccLoggingBillingAccountExclusion_multipleCfg(billingAccount), + }, + { + ResourceName: "google_logging_billing_account_exclusion.basic0", + ImportState: true, + ImportStateVerify: true, + }, + { + ResourceName: "google_logging_billing_account_exclusion.basic1", + ImportState: true, + ImportStateVerify: true, + }, + { + ResourceName: "google_logging_billing_account_exclusion.basic2", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) } func testAccCheckLoggingBillingAccountExclusionDestroy(s *terraform.State) error { @@ -110,52 +133,7 @@ func testAccCheckLoggingBillingAccountExclusionDestroy(s *terraform.State) error return nil } -func testAccCheckLoggingBillingAccountExclusionExists(n string, exclusion *logging.LogExclusion) resource.TestCheckFunc { - return func(s *terraform.State) error { - attributes, err := getResourceAttributes(n, s) - if err != nil { - return err - } - config := testAccProvider.Meta().(*Config) - - si, err := config.clientLogging.BillingAccounts.Exclusions.Get(attributes["id"]).Do() - if err != nil { - return err - } - *exclusion = *si - - return nil - } -} - -func testAccCheckLoggingBillingAccountExclusion(exclusion *logging.LogExclusion, n string) resource.TestCheckFunc { - return func(s *terraform.State) error { - attributes, err := getResourceAttributes(n, s) - if err != nil { - return err - } - - if exclusion.Description != attributes["description"] { - return fmt.Errorf("mismatch on description: api has %s but client has %s", exclusion.Description, attributes["description"]) - } - - if exclusion.Filter != attributes["filter"] { - return fmt.Errorf("mismatch on filter: api has %s but client has %s", exclusion.Filter, attributes["filter"]) - } - - disabledAttribute, err := toBool(attributes["disabled"]) - if err != nil { - return err - } - if exclusion.Disabled != disabledAttribute { - return fmt.Errorf("mismatch on disabled: api has %t but client has %t", exclusion.Disabled, disabledAttribute) - } - - return nil - } -} - -func testAccLoggingBillingAccountExclusion_basic(exclusionName, description, billingAccount string) string { +func testAccLoggingBillingAccountExclusion_basicCfg(exclusionName, description, billingAccount string) string { return fmt.Sprintf(` resource "google_logging_billing_account_exclusion" "basic" { name = "%s" @@ -165,3 +143,18 @@ resource "google_logging_billing_account_exclusion" "basic" { } `, exclusionName, billingAccount, description, getTestProjectFromEnv()) } + +func testAccLoggingBillingAccountExclusion_multipleCfg(billingAccount string) string { + s := "" + for i := 0; i < 3; i++ { + s += fmt.Sprintf(` +resource "google_logging_billing_account_exclusion" "basic%d" { + name = "%s" + billing_account = "%s" + description = "Basic BillingAccount Logging Exclusion" + filter = "logName=\"projects/%s/logs/compute.googleapis.com%%2Factivity_log\" AND severity>=ERROR" +} +`, i, "tf-test-exclusion-"+acctest.RandString(10), billingAccount, getTestProjectFromEnv()) + } + return s +} diff --git a/google/resource_logging_exclusion.go b/google/resource_logging_exclusion.go index ccc6156e8f4..ab7d63c9d3a 100644 --- a/google/resource_logging_exclusion.go +++ b/google/resource_logging_exclusion.go @@ -54,6 +54,11 @@ func resourceLoggingExclusionCreate(newUpdaterFunc newResourceLoggingExclusionUp id, exclusion := expandResourceLoggingExclusion(d, updater.GetResourceType(), updater.GetResourceId()) + // Logging exclusions don't seem to be able to be mutated in parallel, see + // https://github.com/terraform-providers/terraform-provider-google/issues/4796 + mutexKV.Lock(id.parent()) + defer mutexKV.Unlock(id.parent()) + err = updater.CreateLoggingExclusion(id.parent(), exclusion) if err != nil { return err @@ -97,8 +102,14 @@ func resourceLoggingExclusionUpdate(newUpdaterFunc newResourceLoggingExclusionUp return err } + id, _ := expandResourceLoggingExclusion(d, updater.GetResourceType(), updater.GetResourceId()) exclusion, updateMask := expandResourceLoggingExclusionForUpdate(d) + // Logging exclusions don't seem to be able to be mutated in parallel, see + // https://github.com/terraform-providers/terraform-provider-google/issues/4796 + mutexKV.Lock(id.parent()) + defer mutexKV.Unlock(id.parent()) + err = updater.UpdateLoggingExclusion(d.Id(), exclusion, updateMask) if err != nil { return err @@ -116,6 +127,12 @@ func resourceLoggingExclusionDelete(newUpdaterFunc newResourceLoggingExclusionUp return err } + id, _ := expandResourceLoggingExclusion(d, updater.GetResourceType(), updater.GetResourceId()) + // Logging exclusions don't seem to be able to be mutated in parallel, see + // https://github.com/terraform-providers/terraform-provider-google/issues/4796 + mutexKV.Lock(id.parent()) + defer mutexKV.Unlock(id.parent()) + err = updater.DeleteLoggingExclusion(d.Id()) if err != nil { return err diff --git a/google/resource_logging_folder_exclusion_test.go b/google/resource_logging_folder_exclusion_test.go index afdf89ea992..2f5244cd057 100644 --- a/google/resource_logging_folder_exclusion_test.go +++ b/google/resource_logging_folder_exclusion_test.go @@ -7,30 +7,44 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" - "google.golang.org/api/logging/v2" ) -func TestAccLoggingFolderExclusion_basic(t *testing.T) { +// Logging exclusions don't always work when making parallel requests, so run tests serially +func TestAccLoggingFolderExclusion(t *testing.T) { t.Parallel() + testCases := map[string]func(t *testing.T){ + "basic": testAccLoggingFolderExclusion_basic, + "folderAcceptsFullFolderPath": testAccLoggingFolderExclusion_folderAcceptsFullFolderPath, + "update": testAccLoggingFolderExclusion_update, + "multiple": testAccLoggingFolderExclusion_multiple, + } + + 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 testAccLoggingFolderExclusion_basic(t *testing.T) { org := getTestOrgFromEnv(t) exclusionName := "tf-test-exclusion-" + acctest.RandString(10) folderName := "tf-test-folder-" + acctest.RandString(10) description := "Description " + acctest.RandString(10) - var exclusion logging.LogExclusion - resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckLoggingFolderExclusionDestroy, Steps: []resource.TestStep{ { - Config: testAccLoggingFolderExclusion_basic(exclusionName, description, folderName, "organizations/"+org), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingFolderExclusionExists("google_logging_folder_exclusion.basic", &exclusion), - testAccCheckLoggingFolderExclusion(&exclusion, "google_logging_folder_exclusion.basic"), - ), + Config: testAccLoggingFolderExclusion_basicCfg(exclusionName, description, folderName, "organizations/"+org), }, { ResourceName: "google_logging_folder_exclusion.basic", @@ -41,16 +55,12 @@ func TestAccLoggingFolderExclusion_basic(t *testing.T) { }) } -func TestAccLoggingFolderExclusion_folderAcceptsFullFolderPath(t *testing.T) { - t.Parallel() - +func testAccLoggingFolderExclusion_folderAcceptsFullFolderPath(t *testing.T) { org := getTestOrgFromEnv(t) exclusionName := "tf-test-exclusion-" + acctest.RandString(10) folderName := "tf-test-folder-" + acctest.RandString(10) description := "Description " + acctest.RandString(10) - var exclusion logging.LogExclusion - checkFn := func(s []*terraform.InstanceState) error { loggingExclusionId, err := parseLoggingExclusionId(s[0].ID) if err != nil { @@ -72,10 +82,6 @@ func TestAccLoggingFolderExclusion_folderAcceptsFullFolderPath(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccLoggingFolderExclusion_withFullFolderPath(exclusionName, description, folderName, "organizations/"+org), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingFolderExclusionExists("google_logging_folder_exclusion.full-folder", &exclusion), - testAccCheckLoggingFolderExclusion(&exclusion, "google_logging_folder_exclusion.full-folder"), - ), }, { ResourceName: "google_logging_folder_exclusion.full-folder", @@ -92,9 +98,7 @@ func TestAccLoggingFolderExclusion_folderAcceptsFullFolderPath(t *testing.T) { }) } -func TestAccLoggingFolderExclusion_update(t *testing.T) { - t.Parallel() - +func testAccLoggingFolderExclusion_update(t *testing.T) { org := getTestOrgFromEnv(t) exclusionName := "tf-test-exclusion-" + acctest.RandString(10) folderName := "tf-test-folder-" + acctest.RandString(10) @@ -102,26 +106,21 @@ func TestAccLoggingFolderExclusion_update(t *testing.T) { descriptionBefore := "Basic Folder Logging Exclusion" + acctest.RandString(10) descriptionAfter := "Updated Basic Folder Logging Exclusion" + acctest.RandString(10) - var exclusionBefore, exclusionAfter logging.LogExclusion - resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckLoggingFolderExclusionDestroy, Steps: []resource.TestStep{ { - Config: testAccLoggingFolderExclusion_basic(exclusionName, descriptionBefore, folderName, parent), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingFolderExclusionExists("google_logging_folder_exclusion.basic", &exclusionBefore), - testAccCheckLoggingFolderExclusion(&exclusionBefore, "google_logging_folder_exclusion.basic"), - ), + Config: testAccLoggingFolderExclusion_basicCfg(exclusionName, descriptionBefore, folderName, parent), + }, + { + ResourceName: "google_logging_folder_exclusion.basic", + ImportState: true, + ImportStateVerify: true, }, { - Config: testAccLoggingFolderExclusion_basic(exclusionName, descriptionAfter, folderName, parent), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingFolderExclusionExists("google_logging_folder_exclusion.basic", &exclusionAfter), - testAccCheckLoggingFolderExclusion(&exclusionAfter, "google_logging_folder_exclusion.basic"), - ), + Config: testAccLoggingFolderExclusion_basicCfg(exclusionName, descriptionAfter, folderName, parent), }, { ResourceName: "google_logging_folder_exclusion.basic", @@ -130,19 +129,38 @@ func TestAccLoggingFolderExclusion_update(t *testing.T) { }, }, }) +} - // Description should have changed, but Filter and Disabled should be the same - if exclusionBefore.Description == exclusionAfter.Description { - t.Errorf("Expected Description to change, but it didn't: Description = %#v", exclusionBefore.Description) - } - if exclusionBefore.Filter != exclusionAfter.Filter { - t.Errorf("Expected Filter to be the same, but it differs: before = %#v, after = %#v", - exclusionBefore.Filter, exclusionAfter.Filter) - } - if exclusionBefore.Disabled != exclusionAfter.Disabled { - t.Errorf("Expected Disabled to be the same, but it differs: before = %#v, after = %#v", - exclusionBefore.Disabled, exclusionAfter.Disabled) - } +func testAccLoggingFolderExclusion_multiple(t *testing.T) { + org := getTestOrgFromEnv(t) + folderName := "tf-test-folder-" + acctest.RandString(10) + parent := "organizations/" + org + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckLoggingFolderExclusionDestroy, + Steps: []resource.TestStep{ + { + Config: testAccLoggingFolderExclusion_multipleCfg(folderName, parent), + }, + { + ResourceName: "google_logging_folder_exclusion.basic0", + ImportState: true, + ImportStateVerify: true, + }, + { + ResourceName: "google_logging_folder_exclusion.basic1", + ImportState: true, + ImportStateVerify: true, + }, + { + ResourceName: "google_logging_folder_exclusion.basic2", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) } func testAccCheckLoggingFolderExclusionDestroy(s *terraform.State) error { @@ -164,52 +182,7 @@ func testAccCheckLoggingFolderExclusionDestroy(s *terraform.State) error { return nil } -func testAccCheckLoggingFolderExclusionExists(n string, exclusion *logging.LogExclusion) resource.TestCheckFunc { - return func(s *terraform.State) error { - attributes, err := getResourceAttributes(n, s) - if err != nil { - return err - } - config := testAccProvider.Meta().(*Config) - - si, err := config.clientLogging.Folders.Exclusions.Get(attributes["id"]).Do() - if err != nil { - return err - } - *exclusion = *si - - return nil - } -} - -func testAccCheckLoggingFolderExclusion(exclusion *logging.LogExclusion, n string) resource.TestCheckFunc { - return func(s *terraform.State) error { - attributes, err := getResourceAttributes(n, s) - if err != nil { - return err - } - - if exclusion.Description != attributes["description"] { - return fmt.Errorf("mismatch on description: api has %s but client has %s", exclusion.Description, attributes["description"]) - } - - if exclusion.Filter != attributes["filter"] { - return fmt.Errorf("mismatch on filter: api has %s but client has %s", exclusion.Filter, attributes["filter"]) - } - - disabledAttribute, err := toBool(attributes["disabled"]) - if err != nil { - return err - } - if exclusion.Disabled != disabledAttribute { - return fmt.Errorf("mismatch on disabled: api has %t but client has %t", exclusion.Disabled, disabledAttribute) - } - - return nil - } -} - -func testAccLoggingFolderExclusion_basic(exclusionName, description, folderName, folderParent string) string { +func testAccLoggingFolderExclusion_basicCfg(exclusionName, description, folderName, folderParent string) string { return fmt.Sprintf(` resource "google_logging_folder_exclusion" "basic" { name = "%s" @@ -238,3 +211,24 @@ resource "google_folder" "my-folder" { parent = "%s" }`, exclusionName, description, getTestProjectFromEnv(), folderName, folderParent) } + +func testAccLoggingFolderExclusion_multipleCfg(folderName, folderParent string) string { + s := fmt.Sprintf(` +resource "google_folder" "my-folder" { + display_name = "%s" + parent = "%s" +} +`, folderName, folderParent) + + for i := 0; i < 3; i++ { + s += fmt.Sprintf(` +resource "google_logging_folder_exclusion" "basic%d" { + name = "%s" + folder = "${element(split("/", google_folder.my-folder.name), 1)}" + description = "Basic Folder Logging Exclusion" + filter = "logName=\"projects/%s/logs/compute.googleapis.com%%2Factivity_log\" AND severity>=ERROR" +} +`, i, "tf-test-exclusion-"+acctest.RandString(10), getTestProjectFromEnv()) + } + return s +} diff --git a/google/resource_logging_organization_exclusion_test.go b/google/resource_logging_organization_exclusion_test.go index 65444c1b23a..ebaecc82b25 100644 --- a/google/resource_logging_organization_exclusion_test.go +++ b/google/resource_logging_organization_exclusion_test.go @@ -7,29 +7,42 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" - "google.golang.org/api/logging/v2" ) -func TestAccLoggingOrganizationExclusion_basic(t *testing.T) { +// Logging exclusions don't always work when making parallel requests, so run tests serially +func TestAccLoggingOrganizationExclusion(t *testing.T) { t.Parallel() + testCases := map[string]func(t *testing.T){ + "basic": testAccLoggingOrganizationExclusion_basic, + "update": testAccLoggingOrganizationExclusion_update, + "multiple": testAccLoggingOrganizationExclusion_multiple, + } + + 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 testAccLoggingOrganizationExclusion_basic(t *testing.T) { org := getTestOrgFromEnv(t) exclusionName := "tf-test-exclusion-" + acctest.RandString(10) description := "Description " + acctest.RandString(10) - var exclusion logging.LogExclusion - resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckLoggingOrganizationExclusionDestroy, Steps: []resource.TestStep{ { - Config: testAccLoggingOrganizationExclusion_basic(exclusionName, description, org), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingOrganizationExclusionExists("google_logging_organization_exclusion.basic", &exclusion), - testAccCheckLoggingOrganizationExclusion(&exclusion, "google_logging_organization_exclusion.basic"), - ), + Config: testAccLoggingOrganizationExclusion_basicCfg(exclusionName, description, org), }, { ResourceName: "google_logging_organization_exclusion.basic", @@ -40,34 +53,27 @@ func TestAccLoggingOrganizationExclusion_basic(t *testing.T) { }) } -func TestAccLoggingOrganizationExclusion_update(t *testing.T) { - t.Parallel() - +func testAccLoggingOrganizationExclusion_update(t *testing.T) { org := getTestOrgFromEnv(t) exclusionName := "tf-test-exclusion-" + acctest.RandString(10) descriptionBefore := "Basic Organization Logging Exclusion" + acctest.RandString(10) descriptionAfter := "Updated Basic Organization Logging Exclusion" + acctest.RandString(10) - var exclusionBefore, exclusionAfter logging.LogExclusion - resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckLoggingOrganizationExclusionDestroy, Steps: []resource.TestStep{ { - Config: testAccLoggingOrganizationExclusion_basic(exclusionName, descriptionBefore, org), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingOrganizationExclusionExists("google_logging_organization_exclusion.basic", &exclusionBefore), - testAccCheckLoggingOrganizationExclusion(&exclusionBefore, "google_logging_organization_exclusion.basic"), - ), + Config: testAccLoggingOrganizationExclusion_basicCfg(exclusionName, descriptionBefore, org), }, { - Config: testAccLoggingOrganizationExclusion_basic(exclusionName, descriptionAfter, org), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingOrganizationExclusionExists("google_logging_organization_exclusion.basic", &exclusionAfter), - testAccCheckLoggingOrganizationExclusion(&exclusionAfter, "google_logging_organization_exclusion.basic"), - ), + ResourceName: "google_logging_organization_exclusion.basic", + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccLoggingOrganizationExclusion_basicCfg(exclusionName, descriptionAfter, org), }, { ResourceName: "google_logging_organization_exclusion.basic", @@ -76,19 +82,36 @@ func TestAccLoggingOrganizationExclusion_update(t *testing.T) { }, }, }) +} - // Description should have changed, but Filter and Disabled should be the same - if exclusionBefore.Description == exclusionAfter.Description { - t.Errorf("Expected Description to change, but it didn't: Description = %#v", exclusionBefore.Description) - } - if exclusionBefore.Filter != exclusionAfter.Filter { - t.Errorf("Expected Filter to be the same, but it differs: before = %#v, after = %#v", - exclusionBefore.Filter, exclusionAfter.Filter) - } - if exclusionBefore.Disabled != exclusionAfter.Disabled { - t.Errorf("Expected Disabled to be the same, but it differs: before = %#v, after = %#v", - exclusionBefore.Disabled, exclusionAfter.Disabled) - } +func testAccLoggingOrganizationExclusion_multiple(t *testing.T) { + org := getTestOrgFromEnv(t) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckLoggingOrganizationExclusionDestroy, + Steps: []resource.TestStep{ + { + Config: testAccLoggingOrganizationExclusion_multipleCfg(org), + }, + { + ResourceName: "google_logging_organization_exclusion.basic0", + ImportState: true, + ImportStateVerify: true, + }, + { + ResourceName: "google_logging_organization_exclusion.basic1", + ImportState: true, + ImportStateVerify: true, + }, + { + ResourceName: "google_logging_organization_exclusion.basic2", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) } func testAccCheckLoggingOrganizationExclusionDestroy(s *terraform.State) error { @@ -110,52 +133,7 @@ func testAccCheckLoggingOrganizationExclusionDestroy(s *terraform.State) error { return nil } -func testAccCheckLoggingOrganizationExclusionExists(n string, exclusion *logging.LogExclusion) resource.TestCheckFunc { - return func(s *terraform.State) error { - attributes, err := getResourceAttributes(n, s) - if err != nil { - return err - } - config := testAccProvider.Meta().(*Config) - - si, err := config.clientLogging.Organizations.Exclusions.Get(attributes["id"]).Do() - if err != nil { - return err - } - *exclusion = *si - - return nil - } -} - -func testAccCheckLoggingOrganizationExclusion(exclusion *logging.LogExclusion, n string) resource.TestCheckFunc { - return func(s *terraform.State) error { - attributes, err := getResourceAttributes(n, s) - if err != nil { - return err - } - - if exclusion.Description != attributes["description"] { - return fmt.Errorf("mismatch on description: api has %s but client has %s", exclusion.Description, attributes["description"]) - } - - if exclusion.Filter != attributes["filter"] { - return fmt.Errorf("mismatch on filter: api has %s but client has %s", exclusion.Filter, attributes["filter"]) - } - - disabledAttribute, err := toBool(attributes["disabled"]) - if err != nil { - return err - } - if exclusion.Disabled != disabledAttribute { - return fmt.Errorf("mismatch on disabled: api has %t but client has %t", exclusion.Disabled, disabledAttribute) - } - - return nil - } -} - -func testAccLoggingOrganizationExclusion_basic(exclusionName, description, orgId string) string { +func testAccLoggingOrganizationExclusion_basicCfg(exclusionName, description, orgId string) string { return fmt.Sprintf(` resource "google_logging_organization_exclusion" "basic" { name = "%s" @@ -165,3 +143,18 @@ resource "google_logging_organization_exclusion" "basic" { } `, exclusionName, orgId, description, getTestProjectFromEnv()) } + +func testAccLoggingOrganizationExclusion_multipleCfg(orgId string) string { + s := "" + for i := 0; i < 3; i++ { + s += fmt.Sprintf(` +resource "google_logging_organization_exclusion" "basic%d" { + name = "%s" + org_id = "%s" + description = "Basic Organization Logging Exclusion" + filter = "logName=\"projects/%s/logs/compute.googleapis.com%%2Factivity_log\" AND severity>=ERROR" +} +`, i, "tf-test-exclusion-"+acctest.RandString(10), orgId, getTestProjectFromEnv()) + } + return s +} diff --git a/google/resource_logging_project_exclusion_test.go b/google/resource_logging_project_exclusion_test.go index bcbdd3c08c1..aac52ab0178 100644 --- a/google/resource_logging_project_exclusion_test.go +++ b/google/resource_logging_project_exclusion_test.go @@ -7,15 +7,33 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" - "google.golang.org/api/logging/v2" ) -func TestAccLoggingProjectExclusion_basic(t *testing.T) { +// Logging exclusions don't always work when making parallel requests, so run tests serially +func TestAccLoggingProjectExclusion(t *testing.T) { t.Parallel() - exclusionName := "tf-test-exclusion-" + acctest.RandString(10) + testCases := map[string]func(t *testing.T){ + "basic": testAccLoggingProjectExclusion_basic, + "disablePreservesFilter": testAccLoggingProjectExclusion_disablePreservesFilter, + "update": testAccLoggingProjectExclusion_update, + "multiple": testAccLoggingProjectExclusion_multiple, + } + + 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) + }) + } +} - var exclusion logging.LogExclusion +func testAccLoggingProjectExclusion_basic(t *testing.T) { + exclusionName := "tf-test-exclusion-" + acctest.RandString(10) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -23,10 +41,7 @@ func TestAccLoggingProjectExclusion_basic(t *testing.T) { CheckDestroy: testAccCheckLoggingProjectExclusionDestroy, Steps: []resource.TestStep{ { - Config: testAccLoggingProjectExclusion_basic(exclusionName), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingProjectExclusionExists("google_logging_project_exclusion.basic", &exclusion), - testAccCheckLoggingProjectExclusion(&exclusion, "google_logging_project_exclusion.basic")), + Config: testAccLoggingProjectExclusion_basicCfg(exclusionName), }, { ResourceName: "google_logging_project_exclusion.basic", @@ -37,31 +52,24 @@ func TestAccLoggingProjectExclusion_basic(t *testing.T) { }) } -func TestAccLoggingProjectExclusion_disablePreservesFilter(t *testing.T) { - t.Parallel() - +func testAccLoggingProjectExclusion_disablePreservesFilter(t *testing.T) { exclusionName := "tf-test-exclusion-" + acctest.RandString(10) - var exclusionBefore, exclusionAfter logging.LogExclusion - resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckLoggingProjectExclusionDestroy, Steps: []resource.TestStep{ { - Config: testAccLoggingProjectExclusion_basic(exclusionName), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingProjectExclusionExists("google_logging_project_exclusion.basic", &exclusionBefore), - testAccCheckLoggingProjectExclusion(&exclusionBefore, "google_logging_project_exclusion.basic"), - ), + Config: testAccLoggingProjectExclusion_basicCfg(exclusionName), + }, + { + ResourceName: "google_logging_project_exclusion.basic", + ImportState: true, + ImportStateVerify: true, }, { Config: testAccLoggingProjectExclusion_basicDisabled(exclusionName), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingProjectExclusionExists("google_logging_project_exclusion.basic", &exclusionAfter), - testAccCheckLoggingProjectExclusion(&exclusionAfter, "google_logging_project_exclusion.basic"), - ), }, { ResourceName: "google_logging_project_exclusion.basic", @@ -70,45 +78,26 @@ func TestAccLoggingProjectExclusion_disablePreservesFilter(t *testing.T) { }, }, }) - - // Description and Disabled should have changed, but Filter should be the same - if exclusionBefore.Description == exclusionAfter.Description { - t.Errorf("Expected Description to change, but it didn't: Description = %#v", exclusionBefore.Description) - } - if exclusionBefore.Filter != exclusionAfter.Filter { - t.Errorf("Expected Filter to be the same, but it differs: before = %#v, after = %#v", - exclusionBefore.Filter, exclusionAfter.Filter) - } - if exclusionBefore.Disabled == exclusionAfter.Disabled { - t.Errorf("Expected Disabled to change, but it didn't: Disabled = %#v", exclusionBefore.Disabled) - } } -func TestAccLoggingProjectExclusion_update(t *testing.T) { - t.Parallel() - +func testAccLoggingProjectExclusion_update(t *testing.T) { exclusionName := "tf-test-exclusion-" + acctest.RandString(10) - var exclusionBefore, exclusionAfter logging.LogExclusion - resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckLoggingProjectExclusionDestroy, Steps: []resource.TestStep{ { - Config: testAccLoggingProjectExclusion_basic(exclusionName), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingProjectExclusionExists("google_logging_project_exclusion.basic", &exclusionBefore), - testAccCheckLoggingProjectExclusion(&exclusionBefore, "google_logging_project_exclusion.basic"), - ), + Config: testAccLoggingProjectExclusion_basicCfg(exclusionName), + }, + { + ResourceName: "google_logging_project_exclusion.basic", + ImportState: true, + ImportStateVerify: true, }, { Config: testAccLoggingProjectExclusion_basicUpdated(exclusionName), - Check: resource.ComposeTestCheckFunc( - testAccCheckLoggingProjectExclusionExists("google_logging_project_exclusion.basic", &exclusionAfter), - testAccCheckLoggingProjectExclusion(&exclusionAfter, "google_logging_project_exclusion.basic"), - ), }, { ResourceName: "google_logging_project_exclusion.basic", @@ -117,19 +106,34 @@ func TestAccLoggingProjectExclusion_update(t *testing.T) { }, }, }) +} - // Filter should have changed, but Description and Disabled should be the same - if exclusionBefore.Description != exclusionAfter.Description { - t.Errorf("Expected Description to be the same, but it differs: before = %#v, after = %#v", - exclusionBefore.Description, exclusionAfter.Description) - } - if exclusionBefore.Filter == exclusionAfter.Filter { - t.Errorf("Expected Filter to change, but it didn't: Filter = %#v", exclusionBefore.Filter) - } - if exclusionBefore.Disabled != exclusionAfter.Disabled { - t.Errorf("Expected Disabled to be the same, but it differs: before = %#v, after = %#v", - exclusionBefore.Disabled, exclusionAfter.Disabled) - } +func testAccLoggingProjectExclusion_multiple(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckLoggingProjectExclusionDestroy, + Steps: []resource.TestStep{ + { + Config: testAccLoggingProjectExclusion_multipleCfg(), + }, + { + ResourceName: "google_logging_project_exclusion.basic0", + ImportState: true, + ImportStateVerify: true, + }, + { + ResourceName: "google_logging_project_exclusion.basic1", + ImportState: true, + ImportStateVerify: true, + }, + { + ResourceName: "google_logging_project_exclusion.basic2", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) } func testAccCheckLoggingProjectExclusionDestroy(s *terraform.State) error { @@ -144,59 +148,14 @@ func testAccCheckLoggingProjectExclusionDestroy(s *terraform.State) error { _, err := config.clientLogging.Projects.Exclusions.Get(attributes["id"]).Do() if err == nil { - return fmt.Errorf("project exclusion still exists") + return fmt.Errorf("project exclusion %s still exists", attributes["id"]) } } return nil } -func testAccCheckLoggingProjectExclusionExists(n string, exclusion *logging.LogExclusion) resource.TestCheckFunc { - return func(s *terraform.State) error { - attributes, err := getResourceAttributes(n, s) - if err != nil { - return err - } - config := testAccProvider.Meta().(*Config) - - si, err := config.clientLogging.Projects.Exclusions.Get(attributes["id"]).Do() - if err != nil { - return err - } - *exclusion = *si - - return nil - } -} - -func testAccCheckLoggingProjectExclusion(exclusion *logging.LogExclusion, n string) resource.TestCheckFunc { - return func(s *terraform.State) error { - attributes, err := getResourceAttributes(n, s) - if err != nil { - return err - } - - if exclusion.Description != attributes["description"] { - return fmt.Errorf("mismatch on description: api has %s but client has %s", exclusion.Description, attributes["description"]) - } - - if exclusion.Filter != attributes["filter"] { - return fmt.Errorf("mismatch on filter: api has %s but client has %s", exclusion.Filter, attributes["filter"]) - } - - disabledAttribute, err := toBool(attributes["disabled"]) - if err != nil { - return err - } - if exclusion.Disabled != disabledAttribute { - return fmt.Errorf("mismatch on disabled: api has %t but client has %t", exclusion.Disabled, disabledAttribute) - } - - return nil - } -} - -func testAccLoggingProjectExclusion_basic(name string) string { +func testAccLoggingProjectExclusion_basicCfg(name string) string { return fmt.Sprintf(` resource "google_logging_project_exclusion" "basic" { name = "%s" @@ -223,3 +182,17 @@ resource "google_logging_project_exclusion" "basic" { disabled = true }`, name, getTestProjectFromEnv()) } + +func testAccLoggingProjectExclusion_multipleCfg() string { + s := "" + for i := 0; i < 3; i++ { + s += fmt.Sprintf(` +resource "google_logging_project_exclusion" "basic%d" { + name = "%s" + description = "Basic Project Logging Exclusion" + filter = "logName=\"projects/%s/logs/compute.googleapis.com%%2Factivity_log\" AND severity>=ERROR" +} +`, i, "tf-test-exclusion-"+acctest.RandString(10), getTestProjectFromEnv()) + } + return s +} diff --git a/google/test_utils.go b/google/test_utils.go index 0856e7e8eff..3a9b611ad37 100644 --- a/google/test_utils.go +++ b/google/test_utils.go @@ -3,7 +3,6 @@ package google import ( "fmt" "reflect" - "strconv" "github.com/hashicorp/terraform-plugin-sdk/terraform" ) @@ -80,14 +79,6 @@ func (d *ResourceDiffMock) Clear(key string) error { return nil } -func toBool(attribute string) (bool, error) { - // Handle the case where an unset value defaults to false - if attribute == "" { - return false, nil - } - return strconv.ParseBool(attribute) -} - func checkDataSourceStateMatchesResourceState(dataSourceName, resourceName string) func(*terraform.State) error { return checkDataSourceStateMatchesResourceStateWithIgnores(dataSourceName, resourceName, map[string]struct{}{}) }