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.

Sign up for GitHub

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

r/instance - plan time validations #13033

Merged
merged 7 commits into from
Jun 4, 2020
Merged

r/instance - plan time validations #13033

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
24a6202
use enums
DrFaust92 Apr 27, 2020
f2b037d
delete TestInstanceTenancySchema unit test (?)
DrFaust92 Apr 27, 2020
ae099d5
Update resource_aws_instance.go
DrFaust92 Jun 3, 2020
6b49db7
Update resource_aws_instance.go
DrFaust92 Jun 3, 2020
1b116ac
fmt + fix error handle for `InvalidParameterCombination`
DrFaust92 Jun 4, 2020
e7b5edf
Update resource_aws_instance.go
DrFaust92 Jun 4, 2020
47461b4
Update resource_aws_instance.go
DrFaust92 Jun 4, 2020
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
62 changes: 40 additions & 22 deletions aws/resource_aws_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform-plugin-sdk/helper/hashcode"
"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
Expand Down Expand Up @@ -105,10 +104,11 @@ func resourceAwsInstance() *schema.Resource {
},

"private_ip": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Computed: true,
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Computed: true,
ValidateFunc: validation.IsIPv4Address,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validation means that you cannot pass an empty string and as a result ALL of our EC2 instances that were created with a module which passes an empty string (which is a valid value) when an IP is not specified are now producing diffs.

},

"source_dest_check": {
Expand Down Expand Up @@ -288,7 +288,8 @@ func resourceAwsInstance() *schema.Resource {
Computed: true,
ForceNew: true,
Elem: &schema.Schema{
Type: schema.TypeString,
Type: schema.TypeString,
ValidateFunc: validation.IsIPv6Address,
},
},

Expand All @@ -297,6 +298,11 @@ func resourceAwsInstance() *schema.Resource {
Optional: true,
Computed: true,
ForceNew: true,
ValidateFunc: validation.StringInSlice([]string{
ec2.TenancyDedicated,
ec2.TenancyDefault,
ec2.TenancyHost,
}, false),
},
"host_id": {
Type: schema.TypeString,
Expand Down Expand Up @@ -382,6 +388,13 @@ func resourceAwsInstance() *schema.Resource {
Optional: true,
Computed: true,
ForceNew: true,
ValidateFunc: validation.StringInSlice([]string{
ec2.VolumeTypeStandard,
ec2.VolumeTypeIo1,
ec2.VolumeTypeGp2,
ec2.VolumeTypeSc1,
ec2.VolumeTypeSt1,
}, false),
},

"volume_id": {
Expand Down Expand Up @@ -486,6 +499,13 @@ func resourceAwsInstance() *schema.Resource {
Type: schema.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validation.StringInSlice([]string{
ec2.VolumeTypeStandard,
ec2.VolumeTypeIo1,
ec2.VolumeTypeGp2,
ec2.VolumeTypeSc1,
ec2.VolumeTypeSt1,
}, false),
},

"volume_id": {
Expand Down Expand Up @@ -645,7 +665,7 @@ func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error {
// where a user uses group ids in security_groups for the Default VPC.
// See https://github.com/hashicorp/terraform/issues/3798
if isAWSErr(err, "InvalidParameterValue", "groupId is invalid") {
return fmt.Errorf("Error launching instance, possible mismatch of Security Group IDs and Names. See AWS Instance docs here: %s.\n\n\tAWS Error: %s", "https://terraform.io/docs/providers/aws/r/instance.html", err.(awserr.Error).Message())
return fmt.Errorf("Error launching instance, possible mismatch of Security Group IDs and Names. See AWS Instance docs here: %s.\n\n\tAWS Error: %w", "https://terraform.io/docs/providers/aws/r/instance.html", err)
}
if err != nil {
return fmt.Errorf("Error launching source instance: %s", err)
Expand Down Expand Up @@ -709,7 +729,7 @@ func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error {
if err != nil {
// If the instance was not found, return nil so that we can show
// that the instance is gone.
if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "InvalidInstanceID.NotFound" {
if isAWSErr(err, "InvalidInstanceID.NotFound", "") {
d.SetId("")
return nil
}
Expand Down Expand Up @@ -1075,14 +1095,10 @@ func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error {
},
})
if err != nil {
if ec2err, ok := err.(awserr.Error); ok {
// Tolerate InvalidParameterCombination error in Classic, otherwise
// return the error
if ec2err.Code() != "InvalidParameterCombination" {
return err
}
log.Printf("[WARN] Attempted to modify SourceDestCheck on non VPC instance: %s", ec2err.Message())
if !isAWSErr(err, "InvalidParameterCombination", "") {
DrFaust92 marked this conversation as resolved.
Show resolved Hide resolved
return err
}
log.Printf("[WARN] Attempted to modify SourceDestCheck on non VPC instance: %s", err)
}
}
}
Expand Down Expand Up @@ -1164,7 +1180,7 @@ func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error {

stateConf := &resource.StateChangeConf{
Pending: []string{ec2.InstanceStateNamePending, ec2.InstanceStateNameStopped},
Target: []string{"running"},
Target: []string{ec2.InstanceStateNameRunning},
Refresh: InstanceStateRefreshFunc(conn, d.Id(), []string{ec2.InstanceStateNameTerminated}),
Timeout: d.Timeout(schema.TimeoutUpdate),
Delay: 10 * time.Second,
Expand Down Expand Up @@ -1297,9 +1313,9 @@ func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error {
// Optimization can take hours. e.g. a full 1 TiB drive takes approximately 6 hours to optimize,
// according to https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/monitoring-volume-modifications.html
stateConf := &resource.StateChangeConf{
Pending: []string{"modifying"},
Target: []string{"completed", "optimizing"},
Refresh: VolumeStateRefreshFunc(conn, volumeID, "failed"),
Pending: []string{ec2.VolumeModificationStateModifying},
Target: []string{ec2.VolumeModificationStateCompleted, ec2.VolumeModificationStateOptimizing},
Refresh: VolumeStateRefreshFunc(conn, volumeID, ec2.VolumeModificationStateFailed),
Timeout: d.Timeout(schema.TimeoutUpdate),
Delay: 30 * time.Second,
MinTimeout: 30 * time.Second,
Expand Down Expand Up @@ -2153,7 +2169,7 @@ func awsTerminateInstance(conn *ec2.EC2, id string, timeout time.Duration) error
InstanceIds: []*string{aws.String(id)},
}
if _, err := conn.TerminateInstances(req); err != nil {
if isAWSErr(err, ec2.UnsuccessfulInstanceCreditSpecificationErrorCodeInvalidInstanceIdNotFound, "") {
if isAWSErr(err, "InvalidInstanceID.NotFound", "") {
return nil
}
return err
Expand All @@ -2166,7 +2182,8 @@ func waitForInstanceStopping(conn *ec2.EC2, id string, timeout time.Duration) er
log.Printf("[DEBUG] Waiting for instance (%s) to become stopped", id)

stateConf := &resource.StateChangeConf{
Pending: []string{ec2.InstanceStateNamePending, ec2.InstanceStateNameRunning, ec2.InstanceStateNameShuttingDown, ec2.InstanceStateNameStopped, ec2.InstanceStateNameStopping},
Pending: []string{ec2.InstanceStateNamePending, ec2.InstanceStateNameRunning,
ec2.InstanceStateNameShuttingDown, ec2.InstanceStateNameStopped, ec2.InstanceStateNameStopping},
Target: []string{ec2.InstanceStateNameStopped},
Refresh: InstanceStateRefreshFunc(conn, id, []string{}),
Timeout: timeout,
Expand All @@ -2187,7 +2204,8 @@ func waitForInstanceDeletion(conn *ec2.EC2, id string, timeout time.Duration) er
log.Printf("[DEBUG] Waiting for instance (%s) to become terminated", id)

stateConf := &resource.StateChangeConf{
Pending: []string{ec2.InstanceStateNamePending, ec2.InstanceStateNameRunning, ec2.InstanceStateNameShuttingDown, ec2.InstanceStateNameStopped, ec2.InstanceStateNameStopping},
Pending: []string{ec2.InstanceStateNamePending, ec2.InstanceStateNameRunning,
ec2.InstanceStateNameShuttingDown, ec2.InstanceStateNameStopped, ec2.InstanceStateNameStopping},
Target: []string{ec2.InstanceStateNameTerminated},
Refresh: InstanceStateRefreshFunc(conn, id, []string{}),
Timeout: timeout,
Expand Down
16 changes: 0 additions & 16 deletions aws/resource_aws_instance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2987,22 +2987,6 @@ func testAccCheckStopInstance(instance *ec2.Instance) resource.TestCheckFunc {
}
}

func TestInstanceTenancySchema(t *testing.T) {
actualSchema := resourceAwsInstance().Schema["tenancy"]
expectedSchema := &schema.Schema{
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
}
if !reflect.DeepEqual(actualSchema, expectedSchema) {
t.Fatalf(
"Got:\n\n%#v\n\nExpected:\n\n%#v\n",
actualSchema,
expectedSchema)
}
}

func TestInstanceHostIDSchema(t *testing.T) {
actualSchema := resourceAwsInstance().Schema["host_id"]
expectedSchema := &schema.Schema{
Expand Down