r/aws_vpc_peering_connection_options: Only modify VPC Peering options that have actually changed

aws/internal/service/ec2/errors.go

@@ -12,6 +12,10 @@ const (
 	InvalidGroupNotFound           = "InvalidGroup.NotFound"
 )
 
+const (
+	ErrCodeInvalidVpcPeeringConnectionIDNotFound = "InvalidVpcPeeringConnectionID.NotFound"
+)
+
 const (
 	InvalidVpnGatewayAttachmentNotFound = "InvalidVpnGatewayAttachment.NotFound"
 	InvalidVpnGatewayIDNotFound         = "InvalidVpnGatewayID.NotFound"

aws/internal/service/ec2/finder/finder.go

@@ -72,6 +72,25 @@ func SecurityGroupByID(conn *ec2.EC2, id string) (*ec2.SecurityGroup, error) {
 	return result.SecurityGroups[0], nil
 }
 
+// VpcPeeringConnectionByID returns the VPC peering connection corresponding to the specified identifier.
+// Returns nil and potentially an error if no VPC peering connection is found.
+func VpcPeeringConnectionByID(conn *ec2.EC2, id string) (*ec2.VpcPeeringConnection, error) {
+	input := &ec2.DescribeVpcPeeringConnectionsInput{
+		VpcPeeringConnectionIds: aws.StringSlice([]string{id}),
+	}
+
+	output, err := conn.DescribeVpcPeeringConnections(input)
+	if err != nil {
+		return nil, err
+	}
+
+	if output == nil || len(output.VpcPeeringConnections) == 0 {
+		return nil, nil
+	}
+
+	return output.VpcPeeringConnections[0], nil
+}
+
 // VpnGatewayVpcAttachment returns the attachment between the specified VPN gateway and VPC.
 // Returns nil and potentially an error if no attachment is found.
 func VpnGatewayVpcAttachment(conn *ec2.EC2, vpnGatewayID, vpcID string) (*ec2.VpcAttachment, error) {

aws/internal/service/ec2/waiter/status.go

@@ -2,6 +2,7 @@ package waiter
 
 import (
 	"fmt"
+	"log"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
@@ -206,6 +207,43 @@ func SecurityGroupStatus(conn *ec2.EC2, id string) resource.StateRefreshFunc {
 	}
 }
 
+const (
+	vpcPeeringConnectionStatusNotFound = "NotFound"
+	vpcPeeringConnectionStatusUnknown  = "Unknown"
+)
+
+// VpcPeeringConnectionStatus fetches the VPC peering connection and its status
+func VpcPeeringConnectionStatus(conn *ec2.EC2, vpcPeeringConnectionID string) resource.StateRefreshFunc {
+	return func() (interface{}, string, error) {
+		vpcPeeringConnection, err := finder.VpcPeeringConnectionByID(conn, vpcPeeringConnectionID)
+		if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidVpcPeeringConnectionIDNotFound) {
+			return nil, vpcPeeringConnectionStatusNotFound, nil
+		}
+		if err != nil {
+			return nil, vpcPeeringConnectionStatusUnknown, err
+		}
+
+		// Sometimes AWS just has consistency issues and doesn't see
+		// our peering connection yet. Return an empty state.
+		if vpcPeeringConnection == nil || vpcPeeringConnection.Status == nil {
+			return nil, vpcPeeringConnectionStatusNotFound, nil
+		}
+
+		statusCode := aws.StringValue(vpcPeeringConnection.Status.Code)
+
+		// https://docs.aws.amazon.com/vpc/latest/peering/vpc-peering-basics.html#vpc-peering-lifecycle
+		switch statusCode {
+		case ec2.VpcPeeringConnectionStateReasonCodeFailed:
+			log.Printf("[WARN] VPC Peering Connection (%s): %s: %s", vpcPeeringConnectionID, statusCode, aws.StringValue(vpcPeeringConnection.Status.Message))
+			fallthrough
+		case ec2.VpcPeeringConnectionStateReasonCodeDeleted, ec2.VpcPeeringConnectionStateReasonCodeExpired, ec2.VpcPeeringConnectionStateReasonCodeRejected:
+			return nil, vpcPeeringConnectionStatusNotFound, nil
+		}
+
+		return vpcPeeringConnection, statusCode, nil
+	}
+}
+
 const (
 	attachmentStateNotFound = "NotFound"
 	attachmentStateUnknown  = "Unknown"

aws/resource_aws_vpc_peering_connection.go

@@ -11,6 +11,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter"
 )
 
 func resourceAwsVpcPeeringConnection() *schema.Resource {
@@ -187,21 +188,6 @@ func resourceVPCPeeringConnectionAccept(conn *ec2.EC2, id string) (string, error
 	return aws.StringValue(resp.VpcPeeringConnection.Status.Code), nil
 }
 
-func resourceAwsVpcPeeringConnectionModifyOptions(d *schema.ResourceData, meta interface{}, crossRegionPeering bool) error {
-	conn := meta.(*AWSClient).ec2conn
-
-	req := &ec2.ModifyVpcPeeringConnectionOptionsInput{
-		VpcPeeringConnectionId:            aws.String(d.Id()),
-		AccepterPeeringConnectionOptions:  expandVpcPeeringConnectionOptions(d.Get("accepter").([]interface{}), crossRegionPeering),
-		RequesterPeeringConnectionOptions: expandVpcPeeringConnectionOptions(d.Get("requester").([]interface{}), crossRegionPeering),
-	}
-
-	log.Printf("[DEBUG] Modifying VPC Peering Connection options: %#v", req)
-	_, err := conn.ModifyVpcPeeringConnectionOptions(req)
-
-	return err
-}
-
 func resourceAwsVPCPeeringUpdate(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).ec2conn
 
@@ -230,17 +216,31 @@ func resourceAwsVPCPeeringUpdate(d *schema.ResourceData, meta interface{}) error
 			return fmt.Errorf("Unable to accept VPC Peering Connection: %s", err)
 		}
 		log.Printf("[DEBUG] VPC Peering Connection accept status: %s", statusCode)
+
+		// "OperationNotPermitted: Peering pcx-0000000000000000 is not active. Peering options can be added only to active peerings."
+		if err := vpcPeeringConnectionWaitUntilAvailable(conn, d.Id(), d.Timeout(schema.TimeoutUpdate)); err != nil {
+			return fmt.Errorf("Error waiting for VPC Peering Connection to become available: %s", err)
+		}
 	}
 
 	if d.HasChanges("accepter", "requester") {
 		if statusCode == ec2.VpcPeeringConnectionStateReasonCodeActive || statusCode == ec2.VpcPeeringConnectionStateReasonCodeProvisioning {
 			pc := pcRaw.(*ec2.VpcPeeringConnection)
-			crossRegionPeering := false
-			if aws.StringValue(pc.RequesterVpcInfo.Region) != aws.StringValue(pc.AccepterVpcInfo.Region) {
-				crossRegionPeering = true
+			crossRegionPeering := aws.StringValue(pc.RequesterVpcInfo.Region) != aws.StringValue(pc.AccepterVpcInfo.Region)
+
+			req := &ec2.ModifyVpcPeeringConnectionOptionsInput{
+				VpcPeeringConnectionId: aws.String(d.Id()),
+			}
+			if d.HasChange("accepter") {
+				req.AccepterPeeringConnectionOptions = expandVpcPeeringConnectionOptions(d.Get("accepter").([]interface{}), crossRegionPeering)
 			}
-			if err := resourceAwsVpcPeeringConnectionModifyOptions(d, meta, crossRegionPeering); err != nil {
-				return fmt.Errorf("Error modifying VPC Peering Connection options: %s", err)
+			if d.HasChange("requester") {
+				req.RequesterPeeringConnectionOptions = expandVpcPeeringConnectionOptions(d.Get("requester").([]interface{}), crossRegionPeering)
+			}
+
+			log.Printf("[DEBUG] Modifying VPC Peering Connection options: %s", req)
+			if _, err := conn.ModifyVpcPeeringConnectionOptions(req); err != nil {
+				return fmt.Errorf("error modifying VPC Peering Connection (%s) Options: %s", d.Id(), err)
 			}
 		} else {
 			return fmt.Errorf("Unable to modify peering options. The VPC Peering Connection "+
@@ -249,10 +249,6 @@ func resourceAwsVPCPeeringUpdate(d *schema.ResourceData, meta interface{}) error
 		}
 	}
 
-	if err := vpcPeeringConnectionWaitUntilAvailable(conn, d.Id(), d.Timeout(schema.TimeoutUpdate)); err != nil {
-		return fmt.Errorf("Error waiting for VPC Peering Connection to become available: %s", err)
-	}
-
 	return resourceAwsVPCPeeringRead(d, meta)
 }
 
@@ -269,6 +265,11 @@ func resourceAwsVPCPeeringDelete(d *schema.ResourceData, meta interface{}) error
 		return nil
 	}
 
+	// "InvalidStateTransition: Invalid state transition for pcx-0000000000000000, attempted to transition from failed to deleting"
+	if isAWSErr(err, "InvalidStateTransition", "to deleting") {
+		return nil
+	}
+
 	if err != nil {
 		return fmt.Errorf("Error deleting VPC Peering Connection (%s): %s", d.Id(), err)
 	}
@@ -280,6 +281,19 @@ func resourceAwsVPCPeeringDelete(d *schema.ResourceData, meta interface{}) error
 	return nil
 }
 
+// vpcPeeringConnection returns the VPC peering connection corresponding to the specified identifier.
+// Returns nil if no VPC peering connection is found or the connection has reached a terminal state
+// according to https://docs.aws.amazon.com/vpc/latest/peering/vpc-peering-basics.html#vpc-peering-lifecycle.
+func vpcPeeringConnection(conn *ec2.EC2, vpcPeeringConnectionID string) (*ec2.VpcPeeringConnection, error) {
+	outputRaw, _, err := waiter.VpcPeeringConnectionStatus(conn, vpcPeeringConnectionID)()
+
+	if output, ok := outputRaw.(*ec2.VpcPeeringConnection); ok {
+		return output, err
+	}
+
+	return nil, err
+}
+
 func vpcPeeringConnectionRefreshState(conn *ec2.EC2, id string) resource.StateRefreshFunc {
 	return func() (interface{}, string, error) {
 		resp, err := conn.DescribeVpcPeeringConnections(&ec2.DescribeVpcPeeringConnectionsInput{

aws/resource_aws_vpc_peering_connection_options.go

@@ -3,9 +3,11 @@ package aws
 import (
 	"fmt"
 	"log"
+	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
@@ -33,25 +35,25 @@ func resourceAwsVpcPeeringConnectionOptions() *schema.Resource {
 
 func resourceAwsVpcPeeringConnectionOptionsCreate(d *schema.ResourceData, meta interface{}) error {
 	d.SetId(d.Get("vpc_peering_connection_id").(string))
+
 	return resourceAwsVpcPeeringConnectionOptionsUpdate(d, meta)
 }
 
 func resourceAwsVpcPeeringConnectionOptionsRead(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).ec2conn
 
-	pcRaw, _, err := vpcPeeringConnectionRefreshState(conn, d.Id())()
+	pc, err := vpcPeeringConnection(conn, d.Id())
+
 	if err != nil {
-		return fmt.Errorf("error reading VPC Peering Connection: %s", err)
+		return fmt.Errorf("error reading VPC Peering Connection (%s): %w", d.Id(), err)
 	}
 
-	if pcRaw == nil {
+	if pc == nil {
 		log.Printf("[WARN] VPC Peering Connection (%s) not found, removing from state", d.Id())
 		d.SetId("")
 		return nil
 	}
 
-	pc := pcRaw.(*ec2.VpcPeeringConnection)
-
 	d.Set("vpc_peering_connection_id", pc.VpcPeeringConnectionId)
 
 	if err := d.Set("accepter", flattenVpcPeeringConnectionOptions(pc.AccepterVpcInfo.PeeringOptions)); err != nil {
@@ -67,25 +69,66 @@ func resourceAwsVpcPeeringConnectionOptionsRead(d *schema.ResourceData, meta int
 func resourceAwsVpcPeeringConnectionOptionsUpdate(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).ec2conn
 
-	pcRaw, _, err := vpcPeeringConnectionRefreshState(conn, d.Id())()
+	pc, err := vpcPeeringConnection(conn, d.Id())
+
 	if err != nil {
-		return fmt.Errorf("error reading VPC Peering Connection (%s): %s", d.Id(), err)
+		return fmt.Errorf("error reading VPC Peering Connection (%s): %w", d.Id(), err)
 	}
 
-	if pcRaw == nil {
-		log.Printf("[WARN] VPC Peering Connection (%s) not found, removing from state", d.Id())
-		d.SetId("")
-		return nil
+	if pc == nil {
+		return fmt.Errorf("VPC Peering Connection (%s) not found", d.Id())
 	}
 
-	pc := pcRaw.(*ec2.VpcPeeringConnection)
-
-	crossRegionPeering := false
-	if aws.StringValue(pc.RequesterVpcInfo.Region) != aws.StringValue(pc.AccepterVpcInfo.Region) {
-		crossRegionPeering = true
-	}
-	if err := resourceAwsVpcPeeringConnectionModifyOptions(d, meta, crossRegionPeering); err != nil {
-		return fmt.Errorf("error modifying VPC Peering Connection (%s) Options: %s", d.Id(), err)
+	if d.HasChanges("accepter", "requester") {
+		crossRegionPeering := aws.StringValue(pc.RequesterVpcInfo.Region) != aws.StringValue(pc.AccepterVpcInfo.Region)
+
+		input := &ec2.ModifyVpcPeeringConnectionOptionsInput{
+			VpcPeeringConnectionId: aws.String(d.Id()),
+		}
+		if d.HasChange("accepter") {
+			input.AccepterPeeringConnectionOptions = expandVpcPeeringConnectionOptions(d.Get("accepter").([]interface{}), crossRegionPeering)
+		}
+		if d.HasChange("requester") {
+			input.RequesterPeeringConnectionOptions = expandVpcPeeringConnectionOptions(d.Get("requester").([]interface{}), crossRegionPeering)
+		}
+
+		log.Printf("[DEBUG] Modifying VPC Peering Connection options: %s", input)
+		_, err = conn.ModifyVpcPeeringConnectionOptions(input)
+
+		if err != nil {
+			return fmt.Errorf("error modifying VPC Peering Connection (%s) Options: %w", d.Id(), err)
+		}
+
+		// Retry reading back the modified options to deal with eventual consistency.
+		// Often this is to do with a delay transitioning from pending-acceptance to active.
+		err = resource.Retry(3*time.Minute, func() *resource.RetryError {
+			pc, err = vpcPeeringConnection(conn, d.Id())
+
+			if err != nil {
+				return resource.NonRetryableError(err)
+			}
+
+			if pc == nil {
+				return nil
+			}
+
+			if d.HasChange("accepter") && pc.AccepterVpcInfo != nil {
+				if aws.BoolValue(pc.AccepterVpcInfo.PeeringOptions.AllowDnsResolutionFromRemoteVpc) != aws.BoolValue(input.AccepterPeeringConnectionOptions.AllowDnsResolutionFromRemoteVpc) ||
+					aws.BoolValue(pc.AccepterVpcInfo.PeeringOptions.AllowEgressFromLocalClassicLinkToRemoteVpc) != aws.BoolValue(input.AccepterPeeringConnectionOptions.AllowEgressFromLocalClassicLinkToRemoteVpc) ||
+					aws.BoolValue(pc.AccepterVpcInfo.PeeringOptions.AllowEgressFromLocalVpcToRemoteClassicLink) != aws.BoolValue(input.AccepterPeeringConnectionOptions.AllowEgressFromLocalVpcToRemoteClassicLink) {
+					return resource.RetryableError(fmt.Errorf("VPC Peering Connection (%s) accepter Options not stable", d.Id()))
+				}
+			}
+			if d.HasChange("requester") && pc.RequesterVpcInfo != nil {
+				if aws.BoolValue(pc.RequesterVpcInfo.PeeringOptions.AllowDnsResolutionFromRemoteVpc) != aws.BoolValue(input.RequesterPeeringConnectionOptions.AllowDnsResolutionFromRemoteVpc) ||
+					aws.BoolValue(pc.RequesterVpcInfo.PeeringOptions.AllowEgressFromLocalClassicLinkToRemoteVpc) != aws.BoolValue(input.RequesterPeeringConnectionOptions.AllowEgressFromLocalClassicLinkToRemoteVpc) ||
+					aws.BoolValue(pc.RequesterVpcInfo.PeeringOptions.AllowEgressFromLocalVpcToRemoteClassicLink) != aws.BoolValue(input.RequesterPeeringConnectionOptions.AllowEgressFromLocalVpcToRemoteClassicLink) {
+					return resource.RetryableError(fmt.Errorf("VPC Peering Connection (%s) requester Options not stable", d.Id()))
+				}
+			}
+
+			return nil
+		})
 	}
 
 	return resourceAwsVpcPeeringConnectionOptionsRead(d, meta)