diff --git a/aws/provider_test.go b/aws/provider_test.go index a7ca181ba2e..ee0626ed976 100644 --- a/aws/provider_test.go +++ b/aws/provider_test.go @@ -9,6 +9,7 @@ import ( "regexp" "sort" "strings" + "sync" "testing" "github.com/aws/aws-sdk-go/aws" @@ -76,14 +77,25 @@ var testAccProviderFactories map[string]func() (*schema.Provider, error) // testAccPreCheck(t) must be called before using this provider instance. var testAccProvider *schema.Provider +// testAccProviderConfigure ensures testAccProvider is only configured once +// +// The testAccPreCheck(t) function is invoked for every test and this prevents +// extraneous reconfiguration to the same values each time. However, this does +// not prevent reconfiguration that may happen should the address of +// testAccProvider be errantly reused in ProviderFactories. +var testAccProviderConfigure sync.Once + func init() { testAccProvider = Provider() testAccProviders = map[string]*schema.Provider{ ProviderNameAws: testAccProvider, } + + // Always allocate a new provider instance each invocation, otherwise gRPC + // ProviderConfigure() can overwrite configuration during concurrent testing. testAccProviderFactories = map[string]func() (*schema.Provider, error){ - ProviderNameAws: func() (*schema.Provider, error) { return testAccProvider, nil }, + ProviderNameAws: func() (*schema.Provider, error) { return Provider(), nil }, //nolint:unparam } } @@ -166,23 +178,41 @@ func TestProvider_impl(t *testing.T) { var _ *schema.Provider = Provider() } +// testAccPreCheck verifies and sets required provider testing configuration +// +// This PreCheck function should be present in every acceptance test. It allows +// test configurations to omit a provider configuration with region and ensures +// testing functions that attempt to call AWS APIs are previously configured. +// +// These verifications and configuration are preferred at this level to prevent +// provider developers from experiencing less clear errors for every test. func testAccPreCheck(t *testing.T) { - if os.Getenv("AWS_PROFILE") == "" && os.Getenv("AWS_ACCESS_KEY_ID") == "" { - t.Fatal("AWS_ACCESS_KEY_ID or AWS_PROFILE must be set for acceptance tests") - } - - if os.Getenv("AWS_ACCESS_KEY_ID") != "" && os.Getenv("AWS_SECRET_ACCESS_KEY") == "" { - t.Fatal("AWS_SECRET_ACCESS_KEY must be set for acceptance tests") - } + // Since we are outside the scope of the Terraform configuration we must + // call Configure() to properly initialize the provider configuration. + testAccProviderConfigure.Do(func() { + if os.Getenv("AWS_PROFILE") == "" && os.Getenv("AWS_ACCESS_KEY_ID") == "" { + t.Fatal("AWS_ACCESS_KEY_ID or AWS_PROFILE must be set for acceptance tests") + } - region := testAccGetRegion() - log.Printf("[INFO] Test: Using %s as test region", region) - os.Setenv("AWS_DEFAULT_REGION", region) + if os.Getenv("AWS_ACCESS_KEY_ID") != "" && os.Getenv("AWS_SECRET_ACCESS_KEY") == "" { + t.Fatal("AWS_SECRET_ACCESS_KEY must be set for acceptance tests") + } - err := testAccProvider.Configure(context.Background(), terraform.NewResourceConfigRaw(nil)) - if err != nil { - t.Fatal(err) - } + // Setting the AWS_DEFAULT_REGION environment variable here allows all tests to omit + // a provider configuration with a region. This defaults to us-west-2 for provider + // developer simplicity and has been in the codebase for a very long time. + // + // This handling must be preserved until either: + // * AWS_DEFAULT_REGION is required and checked above (should mention us-west-2 default) + // * Region is automatically handled via shared AWS configuration file and still verified + region := testAccGetRegion() + os.Setenv("AWS_DEFAULT_REGION", region) + + err := testAccProvider.Configure(context.Background(), terraform.NewResourceConfigRaw(nil)) + if err != nil { + t.Fatal(err) + } + }) } // testAccAwsProviderAccountID returns the account ID of an AWS provider