Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource/instance and resource/ebs_volume: error when iops provided for unsupported volume type #14310

Merged
merged 2 commits into from
Jul 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
16 changes: 16 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,13 @@ 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`.
Exceptions to this are in cases where `iops` is set to `null` or `0` such that the Terraform AWS Provider will continue to accept the value regardless of `type`.

## Resource: aws_elastic_transcoder_preset

### video Configuration Block max_frame_rate Argument No Longer Uses 30 Default
Expand Down Expand Up @@ -386,6 +395,13 @@ 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`.
Exceptions to this are in cases where `iops` is set to `null` or `0` such that the Terraform AWS Provider will continue to accept the value regardless of `type`.

## 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