Skip to content

Commit

Permalink
error when iops provided for unsupported type
Browse files Browse the repository at this point in the history
  • Loading branch information
anGie44 committed Jul 23, 2020
1 parent 0b8d55f commit 0388ace
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 53 deletions.
23 changes: 13 additions & 10 deletions aws/resource_aws_ebs_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,27 +114,30 @@ func resourceAwsEbsVolumeCreate(d *schema.ResourceData, meta interface{}) error
request.OutpostArn = aws.String(value.(string))
}

// IOPs are only valid, and required for, storage type io1. The current minimu
// is 100. Instead of a hard validation we we only apply the IOPs to the
// request if the type is io1, and log a warning otherwise. This allows users
// to "disable" iops. See https://github.com/hashicorp/terraform/pull/4146
// IOPs are only valid, and required for, storage type io1. The current minimum
// is 100. Hard validation in place to return an error if IOPs are provided
// for an unsupported storage type.
// Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/12667
var t string
if value, ok := d.GetOk("type"); ok {
t = value.(string)
request.VolumeType = aws.String(t)
}

iops := d.Get("iops").(int)
if t != ec2.VolumeTypeIo1 && iops > 0 {
log.Printf("[WARN] IOPs is only valid on IO1 storage type for EBS Volumes")
} else if t == ec2.VolumeTypeIo1 {
if iops := d.Get("iops").(int); iops > 0 {
if t != ec2.VolumeTypeIo1 {
if t == "" {
// Volume creation would default to gp2
t = ec2.VolumeTypeGp2
}
return fmt.Errorf("error creating ebs_volume: iops attribute not supported for type %s", t)
}
// We add the iops value without validating it's size, to allow AWS to
// enforce a size requirement (currently 100)
request.Iops = aws.Int64(int64(iops))
}

log.Printf(
"[DEBUG] EBS Volume create opts: %s", request)
log.Printf("[DEBUG] EBS Volume create opts: %s", request)
result, err := conn.CreateVolume(request)
if err != nil {
return fmt.Errorf("Error creating EC2 volume: %s", err)
Expand Down
36 changes: 36 additions & 0 deletions aws/resource_aws_ebs_volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,22 @@ func TestAccAWSEBSVolume_NoIops(t *testing.T) {
})
}

// Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/12667
func TestAccAWSEBSVolume_InvalidIopsForType(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsEbsVolumeConfigWithInvalidIopsForType,
ExpectError: regexp.MustCompile(`error creating ebs_volume: iops attribute not supported for type gp2`),
},
},
})
}

func TestAccAWSEBSVolume_withTags(t *testing.T) {
var v ec2.Volume
resourceName := "aws_ebs_volume.test"
Expand Down Expand Up @@ -749,6 +765,26 @@ resource "aws_ebs_volume" "test" {
}
`

const testAccAwsEbsVolumeConfigWithInvalidIopsForType = `
data "aws_availability_zones" "available" {
state = "available"
filter {
name = "opt-in-status"
values = ["opt-in-not-required"]
}
}
resource "aws_ebs_volume" "test" {
availability_zone = "${data.aws_availability_zones.available.names[0]}"
size = 10
iops = 100
tags = {
Name = "TerraformTest"
}
}
`

func testAccAwsEbsVolumeConfigOutpost() string {
return `
data "aws_outposts_outposts" "test" {}
Expand Down
60 changes: 37 additions & 23 deletions aws/resource_aws_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -592,11 +592,11 @@ func resourceAwsInstance() *schema.Resource {
}

func iopsDiffSuppressFunc(k, old, new string, d *schema.ResourceData) bool {
// Suppress diff if volume_type is not io1
// Suppress diff if volume_type is not io1 and iops is unset or configured as 0
i := strings.LastIndexByte(k, '.')
vt := k[:i+1] + "volume_type"
v := d.Get(vt).(string)
return strings.ToLower(v) != ec2.VolumeTypeIo1
return strings.ToLower(v) != ec2.VolumeTypeIo1 && new == "0"
}

func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error {
Expand Down Expand Up @@ -1367,6 +1367,15 @@ func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error {
}
if d.HasChange("root_block_device.0.iops") {
if v, ok := d.Get("root_block_device.0.iops").(int); ok && v != 0 {
// Enforce IOPs usage with a valid volume type
// Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/12667
if t, ok := d.Get("root_block_device.0.volume_type").(string); ok && t != ec2.VolumeTypeIo1 {
if t == "" {
// Volume defaults to gp2
t = ec2.VolumeTypeGp2
}
return fmt.Errorf("error updating instance: iops attribute not supported for type %s", t)
}
modifyVolume = true
input.Iops = aws.Int64(int64(v))
}
Expand Down Expand Up @@ -1823,13 +1832,17 @@ func readBlockDeviceMappingsFromConfig(d *schema.ResourceData, conn *ec2.EC2) ([

if v, ok := bd["volume_type"].(string); ok && v != "" {
ebs.VolumeType = aws.String(v)
if ec2.VolumeTypeIo1 == strings.ToLower(v) {
// Condition: This parameter is required for requests to create io1
// volumes; it is not used in requests to create gp2, st1, sc1, or
// standard volumes.
// See: http://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_EbsBlockDevice.html
if v, ok := bd["iops"].(int); ok && v > 0 {
ebs.Iops = aws.Int64(int64(v))
if iops, ok := bd["iops"].(int); ok && iops > 0 {
if ec2.VolumeTypeIo1 == strings.ToLower(v) {
// Condition: This parameter is required for requests to create io1
// volumes; it is not used in requests to create gp2, st1, sc1, or
// standard volumes.
// See: http://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_EbsBlockDevice.html
ebs.Iops = aws.Int64(int64(iops))
} else {
// Enforce IOPs usage with a valid volume type
// Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/12667
return nil, fmt.Errorf("error creating resource: iops attribute not supported for ebs_block_device with volume_type %s", v)
}
}
}
Expand Down Expand Up @@ -1885,18 +1898,20 @@ func readBlockDeviceMappingsFromConfig(d *schema.ResourceData, conn *ec2.EC2) ([

if v, ok := bd["volume_type"].(string); ok && v != "" {
ebs.VolumeType = aws.String(v)
}

if v, ok := bd["iops"].(int); ok && v > 0 && aws.StringValue(ebs.VolumeType) == ec2.VolumeTypeIo1 {
// Only set the iops attribute if the volume type is io1. Setting otherwise
// can trigger a refresh/plan loop based on the computed value that is given
// from AWS, and prevent us from specifying 0 as a valid iops.
// See https://github.com/hashicorp/terraform/pull/4146
// See https://github.com/hashicorp/terraform/issues/7765
ebs.Iops = aws.Int64(int64(v))
} else if v, ok := bd["iops"].(int); ok && v > 0 && aws.StringValue(ebs.VolumeType) != ec2.VolumeTypeIo1 {
// Message user about incompatibility
log.Print("[WARN] IOPs is only valid on IO1 storage type for EBS Volumes")
if iops, ok := bd["iops"].(int); ok && iops > 0 {
if ec2.VolumeTypeIo1 == strings.ToLower(v) {
// Only set the iops attribute if the volume type is io1. Setting otherwise
// can trigger a refresh/plan loop based on the computed value that is given
// from AWS, and prevent us from specifying 0 as a valid iops.
// See https://github.com/hashicorp/terraform/pull/4146
// See https://github.com/hashicorp/terraform/issues/7765
ebs.Iops = aws.Int64(int64(iops))
} else {
// Enforce IOPs usage with a valid volume type
// Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/12667
return nil, fmt.Errorf("error creating resource: iops attribute not supported for root_block_device with volume_type %s", v)
}
}
}

if dn, err := fetchRootDeviceName(d.Get("ami").(string), conn); err == nil {
Expand Down Expand Up @@ -2064,8 +2079,7 @@ type awsInstanceOpts struct {
MetadataOptions *ec2.InstanceMetadataOptionsRequest
}

func buildAwsInstanceOpts(
d *schema.ResourceData, meta interface{}) (*awsInstanceOpts, error) {
func buildAwsInstanceOpts(d *schema.ResourceData, meta interface{}) (*awsInstanceOpts, error) {
conn := meta.(*AWSClient).ec2conn

instanceType := d.Get("instance_type").(string)
Expand Down
59 changes: 40 additions & 19 deletions aws/resource_aws_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,22 @@ func TestAccAWSInstance_EbsBlockDevice_KmsKeyArn(t *testing.T) {
})
}

// Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/12667
func TestAccAWSInstance_EbsBlockDevice_InvalidIopsForVolumeType(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccCheckInstanceConfigEBSBlockDeviceInvalidIops,
ExpectError: regexp.MustCompile(`error creating resource: iops attribute not supported for ebs_block_device with volume_type gp2`),
},
},
})
}

func TestAccAWSInstance_RootBlockDevice_KmsKeyArn(t *testing.T) {
var instance ec2.Instance
kmsKeyResourceName := "aws_kms_key.test"
Expand Down Expand Up @@ -449,30 +465,19 @@ func TestAccAWSInstance_GP2IopsDevice(t *testing.T) {
})
}

// TestAccAWSInstance_GP2WithIopsValue updated in v3.0.0
// to account for apply-time validation of the root_block_device.iops attribute for supported volume types
// Reference: https://github.com/terraform-providers/terraform-provider-aws/pull/14310
func TestAccAWSInstance_GP2WithIopsValue(t *testing.T) {
var v ec2.Instance
resourceName := "aws_instance.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
IDRefreshName: resourceName,
IDRefreshIgnore: []string{"ephemeral_block_device", "user_data"},
Providers: testAccProviders,
CheckDestroy: testAccCheckInstanceDestroy,
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccInstanceGP2WithIopsValue,
Check: testAccCheckInstanceExists(resourceName, &v),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccInstanceGP2WithIopsValue,
PlanOnly: true,
ExpectNonEmptyPlan: false,
Config: testAccInstanceGP2WithIopsValue,
ExpectError: regexp.MustCompile(`error creating resource: iops attribute not supported for root_block_device with volume_type gp2`),
},
},
})
Expand Down Expand Up @@ -4000,6 +4005,22 @@ resource "aws_instance" "test" {
}
`

var testAccCheckInstanceConfigEBSBlockDeviceInvalidIops = composeConfig(testAccAwsEc2InstanceAmiWithEbsRootVolume,
`
resource "aws_instance" "test" {
ami = data.aws_ami.ami.id
instance_type = "m3.medium"
ebs_block_device {
device_name = "/dev/sdc"
volume_size = 10
volume_type = "gp2"
iops = 100
}
}
`)

const testAccCheckInstanceConfigWithVolumeTags = `
resource "aws_instance" "test" {
ami = "ami-55a7ea65"
Expand Down
14 changes: 14 additions & 0 deletions website/docs/guides/version-3-upgrade.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ Upgrade topics:
- [Resource: aws_api_gateway_method_settings](#resource-aws_api_gateway_method_settings)
- [Resource: aws_autoscaling_group](#resource-aws_autoscaling_group)
- [Resource: aws_dx_gateway](#resource-aws_dx_gateway)
- [Resource: aws_ebs_volume](#resource-aws_ebs_volume)
- [Resource: aws_elastic_transcoder_preset](#resource-aws_elastic_transcoder_preset)
- [Resource: aws_emr_cluster](#resource-aws_emr_cluster)
- [Resource: aws_instance](#resource-aws_instance)
- [Resource: aws_lambda_alias](#resource-aws_lambda_alias)
- [Resource: aws_launch_template](#resource-aws_launch_template)
- [Resource: aws_lb_listener_rule](#resource-aws_lb_listener_rule)
Expand Down Expand Up @@ -246,6 +248,12 @@ resource "aws_autoscaling_group" "example"{

Previously when importing the `aws_dx_gateway` resource with the [`terraform import` command](/docs/commands/import.html), the Terraform AWS Provider would automatically attempt to import an associated `aws_dx_gateway_association` resource(s) as well. This automatic resource import has been removed. Use the [`aws_dx_gateway_association` resource import](/docs/providers/aws/r/dx_gateway_association.html#import) to import those resources separately.

## Resource: aws_ebs_volume

### iops Argument Apply-Time Validation

Previously when the `iops` argument was configured with a `type` other than `io1` (either explicitly or omitted, indicating the default type `gp2`), the Terraform AWS Provider would automatically disregard the value provided to `iops` as it is only configurable for the `io1` volume type per the AWS EC2 API. This behavior has changed such that the Terraform AWS Provider will instead return an error at apply time indicating an `iops` value is invalid for types other than `io1`.

## Resource: aws_elastic_transcoder_preset

### video Configuration Block max_frame_rate Argument No Longer Uses 30 Default
Expand Down Expand Up @@ -386,6 +394,12 @@ resource "aws_emr_cluster" "example" {
}
```

## Resource: aws_instance

### ebs_block_device.iops and root_block_device.iops Argument Apply-Time Validations

Previously when the `iops` argument was configured in either the `ebs_block_device` or `root_block_device` configuration block, the Terraform AWS Provider would automatically disregard the value provided to `iops` if the `type` argument was also configured with a value other than `io1` (either explicitly or omitted, indicating the default type `gp2`) as `iops` are only configurable for the `io1` volume type per the AWS EC2 API. This behavior has changed such that the Terraform AWS Provider will instead return an error at apply time indicating an `iops` value is invalid for volume types other than `io1`.

## Resource: aws_lambda_alias

### Import No Longer Converts Function Name to ARN
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/ebs_volume.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ The following arguments are supported:

* `availability_zone` - (Required) The AZ where the EBS volume will exist.
* `encrypted` - (Optional) If true, the disk will be encrypted.
* `iops` - (Optional) The amount of IOPS to provision for the disk.
* `iops` - (Optional) The amount of IOPS to provision for the disk. Only valid for `type` of `io1`.
* `multi_attach_enabled` - (Optional) Specifies whether to enable Amazon EBS Multi-Attach. Multi-Attach is supported exclusively on `io1` volumes.
* `size` - (Optional) The size of the drive in GiBs.
* `snapshot_id` (Optional) A snapshot to base the EBS volume off of.
Expand Down

0 comments on commit 0388ace

Please sign in to comment.