providers/aws: Validate IOPs for EBS Volumes

[catsby]

IOPs must be an integer.
The range varies by instance size (docs here)pl, so we simply validate that it's an integer.
IOPs is also not valid for any type that is not io1. Unfortunately I don't believe we can check for that in the plan phase, but we validate that in the apply phase here

Also, a minor fix to the docs: us-west-1a does not exist

[catsby]

Fixes #4146 #2298

[billputer]

Just to clarify, does this still allow passing in a false value to iops?

Currently we have a module that accepts two arguments: storage_type and iops. We use this with both gp2 and io1. For example:

storage_type = "gp2"
iops = false

storage_type = "io1"
iops = 200

Basically we need our module to accept both gp2 and io1 storage types. My intuition is that d.GetOk("iops") will return the "false" value and fail validation.

[catsby]

Just to clarify, does this still allow passing in a false value to oops?

Sorry, no :(

iops is an integer value. Passing false doesn't fit here. As the PR stands, supplying iops with type of gp2 will result in an error. But maybe it should just ignore the iops and log a warning? cc @phinze / @jen20 for thoughts on that

[billputer]

The underlying problem that I'm dealing with is that there's I haven't found away in Terraform to conditionally set an argument in a module. That is, only set the "iops" if it's passed in to the module. You can't allow both (storage_type = gp2) and (storage_type = io1, iops = 200).

That's more an issue with Terraform than it is with this PR, which does the sensible thing. Unfortunately it probably means I'll have to have two modules, one for gp2 and one for io1, unless I can find a better workaround.

[phinze]

@catsby I think the zero value here can just skip the validation so that this would work:

storage_type = "gp2"
iops = 0

storage_type = "io1"
iops = 200

I'd actually expect us to have some sort of gp2 exercising acctest that would be failing now w/ an iops validation error. We should probably add one if not.

[catsby]

Agreed. I'll remove the strict validation. @billputer would iops = 0 be sufficient for "false" for your use case?

[billputer]

@catsby Yup, that should do the trick.

[catsby]

@billputer had to tweak it some, but I think this should work for you. Any chance you can try it out? Otherwise I'll just nag @phinze 😄

[jen20]

This LGTM (assuming we don't want to wait to fix this with whatever @phinze is cooking up for resource-level validation!)

[catsby]

don't know, @phinze: should I delay?

[phinze]

Let's merge this as-is and we can follow up once ResourceValidateFunc finally lands. The conditional read stuff is still going to be valid no matter what we do for validation.

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