Tech Debt: Replace Hardcoded Testing us-east-1 Provider Configurations With Aliased Provider Configuration

[bflad]

Description

Previously, certain test configurations that required execution against the us-east-1 region would either hardcode a provider configuration for the us-east-1 region, e.g.

provider "aws" {
  region = "us-east-1"
}

Or would reset the AWS_DEFAULT_REGION environment variable in the acceptance test function, e.g.

oldvar := os.Getenv("AWS_DEFAULT_REGION")
os.Setenv("AWS_DEFAULT_REGION", "us-east-1")
defer os.Setenv("AWS_DEFAULT_REGION", oldvar)

While this works for running single acceptance tests locally and concurrently in our TeamCity CI environment due to its usage of https://github.com/jen20/teamcity-go-test (which spins up separate provider processes per test), this breaks local testing with go test -parallel higher than one and likely would not work in a future CircleCI environment unless we create a separate testing handler like teamcity-go-test.

Some scenarios where this us-east-1 provider configuration needs to occur includes:

• ACM Certificates for API Gateway Domain Names (EDGE endpoints)
• ACM Certificates for CloudFront Distribution viewer certificates
• ACM Certificates for Cognito User Pool Domains
• Cost and Usage Reporting service (skip for now)
• Pricing service (skip for now)

Notably, most of our EC2-Classic specific testing also uses similar patterns to the above, however we will treat these as a separate, special configuration so the EC2-Classic region can be defined via its own environment variable.

Test Updates

NOTE: As mentioned above, testing configurations for EC2-Classic will require a separate update.

To update existing tests, the following needs to occur:

• Remove any AWS_DEFAULT_REGION handling from the acceptance test function
• Add var providers []*schema.Provider to the acceptance test function
• Switch Providers: testAccProviders to ProviderFactories: testAccProviderFactories(&providers)
• Add Config to any ImportState: true TestStep (matching previous step Config)
• Remove any hardcoded provider "aws" {...} from test configuration
• Add testAccUsEast1RegionProviderConfig() to test configuration
• Add provider = "aws.us-east-1" to any test configuration resources requiring the explicit region setting

References

• testAccUsEast1RegionProviderConfig() added via: tests/resource/aws_cloudfront_distribution: Remove extraneous aliases configurations and create aliased us-east-1 provider configuration #8314

[bflad]

While the above handling works well for the ACM certificate resource handling, after further investigation of some of the testing, using ProviderFactories creates challenges in places where testAccProvider.Meta() is hardcoded. Creating wrapper WithProvider() functions seems like unnecessary boilerplate handling in these cases. This will require some additional thought, e.g. creating a new top level provider variable that is initialized to the region (call it testAccProviderUsEast1), but we will want to ensure we are not expanding these provider variables too much either to the point that Exists/CheckDestroy testing is difficult.

[ewbankkit]

@bflad I'm looking at cleaning up the aws_instance EC2-Classic acceptance test for #4987.
You mention

however we will treat these as a separate, special configuration so the EC2-Classic region can be defined via its own environment variable

Would something like

func TestAccAWSInstance_inEc2Classic(t *testing.T) {
	key := "EC2_CLASSIC_REGION"
	ec2ClassicRegion := os.Getenv(key)
	if ec2ClassicRegion == "" {
		t.Skipf("%s must be set to run EC2-Classic acceptance tests", key)
	}

	oldvar := os.Getenv("AWS_DEFAULT_REGION")
	os.Setenv("AWS_DEFAULT_REGION", ec2ClassicRegion)
	defer os.Setenv("AWS_DEFAULT_REGION", oldvar)

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSInstance_inEc2Classic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAWSInstance_inEc2Classic -timeout 120m
=== RUN   TestAccAWSInstance_inEc2Classic
--- SKIP: TestAccAWSInstance_inEc2Classic (0.00s)
    resource_aws_instance_test.go:205: EC2_CLASSIC_REGION must be set to run EC2-Classic acceptance tests
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	0.084s
$ EC2_CLASSIC_REGION=us-east-1 make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSInstance_inEc2Classic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAWSInstance_inEc2Classic -timeout 120m
=== RUN   TestAccAWSInstance_inEc2Classic
=== PAUSE TestAccAWSInstance_inEc2Classic
=== CONT  TestAccAWSInstance_inEc2Classic
--- PASS: TestAccAWSInstance_inEc2Classic (17.92s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	18.026s

be what you were thinking of?

In all (2) of the acceptance tests that I looked at so far simply adding a PreCheck for EC2-Classic

PreCheck: func() {
	testAccPreCheck(t)
	testAccEC2ClassicPreCheck(t)
},

seems sufficient, resulting in a skipped test like

=== CONT  TestAccAWSInstance_inEc2Classic
--- SKIP: TestAccAWSInstance_inEc2Classic (2.07s)
    provider_test.go:257: This test can only run in EC2 Classic, platforms available in us-west-2: ["VPC"]

if a region doesn't support EC2-Classic.

[ewbankkit]

Addressed for aws_pinpoint_app in #11368.

[ghost]

This has been released in version 3.16.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!