Simplify Validation Functions in validators.go with Terraform helper/validation Functions

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

The Terraform AWS Provider has a long history of adding custom validation functions used with Terraform resource schema ValidateFunc. Many times, the patterns discovered throughout these custom validation functions in the provider have influenced the creation of upstream Terraform Provider SDK functions (currently living in the Terraform core helper/validation package). With organic growth, the addition of these custom functions have created large and sometimes difficult to maintain aws/validators.go and aws/validators_test.go files. Much of the validation logic can be replaced with upstream functions to simplify the implementations.

Affected Resource(s)

Once design work is completed, this will be converted into a list. Each resource's validation should be individually updated to ease acceptance testing.

Code Examples

// Existing custom validation function
func validateDocDBParamGroupName(v interface{}, k string) (ws []string, errors []error) {
	value := v.(string)
	if !regexp.MustCompile(``).MatchString(value) {
		errors = append(errors, fmt.Errorf(
			" %q", k))
	}
	if !regexp.MustCompile(``).MatchString(value) {
		errors = append(errors, fmt.Errorf(
			"first character of parameter group %q must be a letter", k))
	}
	if regexp.MustCompile(``).MatchString(value) {
		errors = append(errors, fmt.Errorf(
			"parameter group %q cannot contain two consecutive hyphens", k))
	}
	if regexp.MustCompile(``).MatchString(value) {
		errors = append(errors, fmt.Errorf(
			"parameter group %q cannot end with a hyphen", k))
	}
	if len(value) > 255 {
		errors = append(errors, fmt.Errorf(
			"parameter group %q cannot be greater than 255 characters", k))
	}
	return
}

// Potential new implementation in resource schema or updated custom validation function
ValidateFunc: validation.All(
	validation.StringLenBetween(0, 255),
	validation.StringMatch(regexp.MustCompile(`^[a-z]`), "must begin with a lowercase letter"),
	validation.StringMatch(regexp.MustCompile(`--`), "must not contain two consecutive hyphens"),
	validation.StringMatch(regexp.MustCompile(`-$`), "must not end with a hyphen"),
	validation.StringMatch(regexp.MustCompile(`^[a-z0-9-]+$`), "must contain only lowercase letters, numbers, and hyphens"),
),

[ewbankkit]

Related:

• Tech debt: Replace 'validateAny' and 'validateIntegerInSlice' with upstream 'validation.Any' and 'validation.IntInSlice' #7906

[jukie]

Could these also be used to simplify resource acceptance tests or is the proposal to remove things like below?

For example in a recent pr I spent a non-trivial amount of time hacking together a valid ExpectError string. I'll admit I'm still getting used to what's available but I imagine there must be a better way. I'm sure I can figure something out but if there's an agreed upon code style I'd rather use it.

func TestAccAWSTransferUser_UserName_Validation(t *testing.T) {
....
			{
				Config:      testAccAWSTransferUserName_validation(acctest.RandString(2)),
				ExpectError: regexp.MustCompile(`Invalid "user_name": must be between 3 and 32 alphanumeric or special characters hyphen and underscore. However, "user_name" cannot begin with a hyphen`),
			},
....
}

[bflad]

@jukie I think once we start seeing patterns of validation being used, we can likely start creating constants for the error messages that can be shared between the code and testing

[github-actions]

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

[github-actions]

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.