Implement Common ErrorCheck for Unsupported Operations

[bflad]

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 other comments that do not add relevant new information or questions, 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

The resource/TestCase.ErrorCheck functionality was recently added to the Terraform Plugin SDK which provides a hook after the terraform apply operation to do something with an error before it is potentially returned, e.g. set the test to skip instead of failing.

While there is no global AWS service API standard for the error codes and messages relating to functionality that is not implemented, there are some common ones which should be safe to apply to any test. Given this, it would be great to introduce a common ErrorCheck that can be added and documented.

Something like this should get us started:

// testAccErrorCheck performs common checks for all acceptance tests
//
// This includes skipping common error codes and messages for unsupported functionality.
func testAccErrorCheck(t *testing.T) resource.ErrorCheckFunc {
	return testAccErrorCheckSkipMessagesContaining(t,
		"UnknownOperationException",
		"UnsupportedOperation",
	)
}

Although it should probably use ErrorCheck helpers based on tfawserr functionality instead of string matching due to the various error permutations, such as those seen in testAccPreCheckSkipError and testSweepSkipSweepError:

	if isAWSErr(err, "UnknownOperationException", "") {
		return true
	}
	if isAWSErr(err, "UnsupportedOperation", "") {
		return true
	}
	if isAWSErr(err, "InvalidInputException", "Unknown operation") {
		return true
	}
	if isAWSErr(err, "InvalidAction", "is not valid") {
		return true
	}
	if isAWSErr(err, "InvalidAction", "Unavailable Operation") {
		return true
	}

We should likely be cautious to not implement all service-specific error checks in this one function, since it could get very large. We could consider an All() type ErrorCheckFunc to combine various checks if necessary.

Definition of Done

• Initial testAccErrorCheck implementation, considering AWS Provider specific tfawserr helper functions and the above 5 common error codes and messages
• Update Contributing Guide documentation to include ErrorCheck in all acceptance test examples
• Implementation of ErrorCheck to at least one known test failure to show it now is skipped (see references below)

References

• tests/resource/sagemaker_model_package Make test GovCloud compatible #17549
• tests/resource/aws_vpc: ClassicLink Test Failures #17460
• tests/resource/aws_instance: GovCloud (US) Failures #17412
• tests/resource/sagemaker_image_version: Make test GovCloud compatible #17200
• tests/resource/sagemaker_user_profile: Make test GovCloud compatible #17199
• tests/resource/sagemaker_feature_group: Make test GovCloud compatible #17158

[YakDriver]

Are we sure about this one? Seems like it could hoover up some other stuff.

if isAWSErr(err, "InvalidAction", "is not valid") {
  return true
}

[bflad]

I guess we can see how often in comes up in practice since it is not a common error code, so might be fine to skip for now.

[bflad]

Ah ha, here one is -- at least EC2 is the reason for that:

=== CONT  TestAccAwsEc2ClientVpn_serial/NetworkAssociation_basic
    resource_aws_ec2_client_vpn_network_association_test.go:105: Step 1/2 error: Error running apply: exit status 1

        Error: Error creating Client VPN endpoint: InvalidAction: The action CreateClientVpnEndpoint is not valid for this web service.

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