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/aws_route: Support changing network_interface_id #2353

Closed
wants to merge 2 commits into from
Closed
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
60 changes: 38 additions & 22 deletions aws/resource_aws_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,18 +276,33 @@ func resourceAwsRouteSetResourceData(d *schema.ResourceData, route *ec2.Route) {

func resourceAwsRouteUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn
replaceOpts, err := prepareRouteUpdate(d)
if err != nil {
return err
}
log.Printf("[DEBUG] Route replace config: %s", replaceOpts)

// Replace the route
_, err = conn.ReplaceRoute(replaceOpts)
if err != nil {
return err
}

return nil
}

func prepareRouteUpdate(d *schema.ResourceData) (*ec2.ReplaceRouteInput, error) {
var numTargets int
var setTarget string

allowedTargets := []string{
"egress_only_gateway_id",
"gateway_id",
"nat_gateway_id",
"network_interface_id",
"instance_id",
"network_interface_id",
"vpc_peering_connection_id",
}
replaceOpts := &ec2.ReplaceRouteInput{}

// Check if more than 1 target is specified
for _, target := range allowedTargets {
Expand All @@ -297,20 +312,28 @@ func resourceAwsRouteUpdate(d *schema.ResourceData, meta interface{}) error {
}
}

switch setTarget {
//instance_id is a special case due to the fact that AWS will "discover" the network_interace_id
//when it creates the route and return that data. In the case of an update, we should ignore the
//existing network_interface_id
case "instance_id":
if numTargets > 2 || (numTargets == 2 && len(d.Get("network_interface_id").(string)) == 0) {
return routeTargetValidationError
}
default:
if numTargets > 1 {
return routeTargetValidationError
if numTargets > 1 {
if setTarget == "network_interface_id" && numTargets == 2 {
// instance_id and network_interface_id are both computed from each
// other. Specifying either in config will cause the other to be
// computed by resourceAwsRouteSetResourceData, so they will both show up
// as targets.
if len(d.Get("instance_id").(string)) == 0 {
return nil, routeTargetValidationError
}
// We check network_interface_id after instance_id in allowedTargets, so
// we will always prefer network_interface_id. If instance_id is the one
// that has changed from the instance state, we should update that
// instead.
if d.HasChange("instance_id") && !d.HasChange("network_interface_id") {
setTarget = "instance_id"
}
} else {
return nil, routeTargetValidationError
}
}

var replaceOpts *ec2.ReplaceRouteInput
// Formulate ReplaceRouteInput based on the target type
switch setTarget {
case "gateway_id":
Expand Down Expand Up @@ -350,17 +373,10 @@ func resourceAwsRouteUpdate(d *schema.ResourceData, meta interface{}) error {
VpcPeeringConnectionId: aws.String(d.Get("vpc_peering_connection_id").(string)),
}
default:
return fmt.Errorf("An invalid target type specified: %s", setTarget)
return nil, fmt.Errorf("An invalid target type specified: %s", setTarget)
}
log.Printf("[DEBUG] Route replace config: %s", replaceOpts)

// Replace the route
_, err := conn.ReplaceRoute(replaceOpts)
if err != nil {
return err
}

return nil
return replaceOpts, nil
}

func resourceAwsRouteDelete(d *schema.ResourceData, meta interface{}) error {
Expand Down
47 changes: 47 additions & 0 deletions aws/resource_aws_route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/terraform"
)

Expand Down Expand Up @@ -253,6 +254,52 @@ func TestAccAWSRoute_doesNotCrashWithVPCEndpoint(t *testing.T) {
})
}

func TestAWSRoute_instanceIdChange(t *testing.T) {
r := resourceAwsRoute()

state := map[string]string{
"instance_id": "i-1",
"network_interface_id": "eni-1",
}
raw := map[string]interface{}{
"instance_id": "i-2",
}
d := schema.TestResourceDataStateRaw(t, r.Schema, state, raw)
replaceOpts, err := prepareRouteUpdate(d)
if err != nil {
t.Fatal(err)
}
if replaceOpts.NetworkInterfaceId != nil {
t.Error("NetworkInterfaceId changed", replaceOpts)
}
if replaceOpts.InstanceId == nil || *replaceOpts.InstanceId != "i-2" {
t.Error("InstanceId unchanged", replaceOpts)
}
}

func TestAWSRoute_networkInterfaceIdChange(t *testing.T) {
r := resourceAwsRoute()

state := map[string]string{
"instance_id": "i-1",
"network_interface_id": "eni-1",
}
raw := map[string]interface{}{
"network_interface_id": "eni-2",
}
d := schema.TestResourceDataStateRaw(t, r.Schema, state, raw)
replaceOpts, err := prepareRouteUpdate(d)
if err != nil {
t.Fatal(err)
}
if replaceOpts.InstanceId != nil {
t.Error("InstanceId changed", replaceOpts)
}
if replaceOpts.NetworkInterfaceId == nil || *replaceOpts.NetworkInterfaceId != "eni-2" {
t.Error("NetworkInterfaceId unchanged", replaceOpts)
}
}

func testAccCheckAWSRouteExists(n string, res *ec2.Route) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
Expand Down