Tech debt: Use standard cross-account acceptance testing setup

[ewbankkit]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

#7627 added standard cross-account acceptance testing setup functions testAccAlternateRegionProviderConfig(), testAccGetAlternateRegion() etc.
Ensure that these functions are used in acceptance tests for other cross-region resources.

• aws_instance - TestAccAWSInstance_multipleRegions()
• aws_rds_cluster - TestAccAWSRDSCluster_EncryptedCrossRegionReplication()
• aws_route53_zone_association - TestAccAWSRoute53ZoneAssociation_region()
• aws_s3_bucket - Multiple

Related:

• Tech debt: Consistently use 'testAccProviderFunc' in cross-region acceptance tests #7702
• Tech Debt: Replace Hardcoded Testing us-east-1 Provider Configurations With Aliased Provider Configuration #8316
• Fix Hardcoded Region GovCloud Test Failures #15737
• Acceptance testing: Create EC2-Classic alternate test provider #15791

[RickyRajinder]

Hi, I'd like to contribute. Should each hard coded value be replaced with the alternate region functions or should the tests be extended to test on both default and alternate regions?

[ewbankkit]

@RickyRajinder Thanks for volunteering 👏.

Looking at for example testAccAWSClusterConfigEncryptedCrossRegionReplica() I would suggest:

• Removing the 2 hard-coded provider blocks

provider "aws" {
  alias  = "useast1"
  region = "us-east-1"
}
provider "aws" {
  alias  = "uswest2"
  region = "us-west-2"
}

• Replacing

	return fmt.Sprintf(`

with

	return testAccAlternateRegionProviderConfig() + fmt.Sprintf(`

• Removing all provider = "aws.useast1" lines
• Replacing all provider = "aws.uswest2" with provider = "aws.alternate"
• Replacing

data "aws_availability_zones" "us-east-1" {
  provider = "aws.useast1"
}

with

data "aws_availability_zones" "current" {}

and changing associated references

• Adding

data "aws_region" "alternate" {
  provider = "aws.alternate"
}

data "aws_partition" "current" {}

and replacing

  replication_source_identifier = "arn:aws:rds:us-west-2:${data.aws_caller_identity.current.account_id}:cluster:${aws_rds_cluster.test_primary.cluster_identifier}"

with

  replication_source_identifier = "arn:${data.aws_partition.current.partition}:rds:${data.aws_region.alternate.name}:${data.aws_caller_identity.current.account_id}:cluster:${aws_rds_cluster.test_primary.cluster_identifier}"

and in TestAccAWSRDSCluster_EncryptedCrossRegionReplication():

• Replacing

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

with

		PreCheck: func() {
			testAccPreCheck(t)
			testAccMultipleRegionsPreCheck(t)
			testAccAlternateRegionPreCheck(t)
		},

• Replacing

		CheckDestroy:      testAccCheckWithProviders(testAccCheckAWSClusterDestroyWithProvider, &providers),

with

		CheckDestroy:      testAccCheckAWSClusterDestroy,

• Replacing

				Check: resource.ComposeTestCheckFunc(
					testAccCheckAWSClusterExistsWithProvider(resourceName,
						&primaryCluster, testAccAwsRegionProviderFunc("us-west-2", &providers)),
					testAccCheckAWSClusterExistsWithProvider(resourceName2,
						&replicaCluster, testAccAwsRegionProviderFunc("us-east-1", &providers)),
				),

with

				Check: resource.ComposeTestCheckFunc(
					testAccCheckAWSClusterExists(resourceName, &primaryCluster),
					testAccCheckAWSClusterExistsWithProvider(resourceName2,
						&replicaCluster, testAccAwsRegionProviderFunc(testAccGetAlternateRegion(), &providers)),
				),

[ewbankkit]

Also in TestAccAWSRDSCluster_EncryptedCrossRegionReplication(), calling acctest.RandInt() twice is going to cause the import test case to recreate the resources. So save the random value in a variable and pass it to testAccAWSClusterConfigEncryptedCrossRegionReplica().

[ewbankkit]

Re-evaluate once #12277 et al. are merged.

[ewbankkit]

Revisit for v3.x after #14312 is merged.

[ewbankkit]

Closing as this is being/has been addressed via the linked issues.

[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!