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

service/ec2: Additional error handling for VPC Endpoint and VPC Endpoint Service deletion, sweeper fixes for Route Tables, VPC Endpoints, and VPC Endpoint Services #16656

Merged
merged 4 commits into from
Feb 3, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
7 changes: 7 additions & 0 deletions .changelog/16656.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:bug
resource/aws_vpc_endpoint: Return unsuccessful deletion information immediately as an error instead of timing out while waiting for deletion
```

```release-note:bug
resource/aws_vpc_endpoint_service: Return unsuccessful deletion information immediately as an error instead of timing out while waiting for deletion
```
42 changes: 42 additions & 0 deletions aws/internal/service/ec2/errors.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
package ec2

import (
"fmt"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/go-multierror"
)

const (
ErrCodeInvalidParameterValue = "InvalidParameterValue"
)
Expand All @@ -16,6 +24,14 @@ const (
InvalidGroupNotFound = "InvalidGroup.NotFound"
)

const (
ErrCodeInvalidVpcEndpointIdNotFound = "InvalidVpcEndpointId.NotFound"
)

const (
ErrCodeInvalidVpcEndpointServiceIdNotFound = "InvalidVpcEndpointServiceId.NotFound"
)

const (
ErrCodeInvalidVpcPeeringConnectionIDNotFound = "InvalidVpcPeeringConnectionID.NotFound"
)
Expand All @@ -24,3 +40,29 @@ const (
InvalidVpnGatewayAttachmentNotFound = "InvalidVpnGatewayAttachment.NotFound"
InvalidVpnGatewayIDNotFound = "InvalidVpnGatewayID.NotFound"
)

func UnsuccessfulItemError(apiObject *ec2.UnsuccessfulItemError) error {
if apiObject == nil {
return nil
}

return fmt.Errorf("%s: %s", aws.StringValue(apiObject.Code), aws.StringValue(apiObject.Message))
}

func UnsuccessfulItemsError(apiObjects []*ec2.UnsuccessfulItem) error {
var errors *multierror.Error

for _, apiObject := range apiObjects {
if apiObject == nil {
continue
}

err := UnsuccessfulItemError(apiObject.Error)

if err != nil {
errors = multierror.Append(errors, fmt.Errorf("%s: %w", aws.StringValue(apiObject.ResourceId), err))
}
}

return errors.ErrorOrNil()
}
79 changes: 70 additions & 9 deletions aws/resource_aws_route_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import (
"fmt"
"log"
"regexp"
"strings"
"testing"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
Expand All @@ -23,46 +25,105 @@ func init() {

func testSweepRouteTables(region string) error {
client, err := sharedClientForRegion(region)

if err != nil {
return fmt.Errorf("error getting client: %s", err)
return fmt.Errorf("error getting client: %w", err)
}

conn := client.(*AWSClient).ec2conn

var sweeperErrs *multierror.Error

input := &ec2.DescribeRouteTablesInput{}

err = conn.DescribeRouteTablesPages(input, func(page *ec2.DescribeRouteTablesOutput, lastPage bool) bool {
if page == nil {
return !lastPage
}

for _, routeTable := range page.RouteTables {
if routeTable == nil {
continue
}

id := aws.StringValue(routeTable.RouteTableId)
isMainRouteTableAssociation := false

for _, routeTableAssociation := range routeTable.Associations {
if routeTableAssociation == nil {
continue
}

if aws.BoolValue(routeTableAssociation.Main) {
isMainRouteTableAssociation = true
break
}

associationID := aws.StringValue(routeTableAssociation.RouteTableAssociationId)

input := &ec2.DisassociateRouteTableInput{
AssociationId: routeTableAssociation.RouteTableAssociationId,
}

log.Printf("[DEBUG] Deleting Route Table Association: %s", input)
log.Printf("[DEBUG] Deleting EC2 Route Table Association: %s", associationID)
_, err := conn.DisassociateRouteTable(input)

if err != nil {
log.Printf("[ERROR] Error deleting Route Table Association (%s): %s", aws.StringValue(routeTableAssociation.RouteTableAssociationId), err)
sweeperErr := fmt.Errorf("error deleting EC2 Route Table (%s) Association (%s): %w", id, associationID, err)
log.Printf("[ERROR] %s", sweeperErr)
sweeperErrs = multierror.Append(sweeperErrs, sweeperErr)
continue
}
}

if isMainRouteTableAssociation {
log.Printf("[DEBUG] Skipping Main Route Table: %s", aws.StringValue(routeTable.RouteTableId))
for _, route := range routeTable.Routes {
if route == nil {
continue
}

if aws.StringValue(route.GatewayId) == "local" {
continue
}

// Prevent deleting default VPC route for Internet Gateway
// which some testing is still reliant on operating correctly
if strings.HasPrefix(aws.StringValue(route.GatewayId), "igw-") && aws.StringValue(route.DestinationCidrBlock) == "0.0.0.0/0" {
continue
}

input := &ec2.DeleteRouteInput{
DestinationCidrBlock: route.DestinationCidrBlock,
DestinationIpv6CidrBlock: route.DestinationIpv6CidrBlock,
RouteTableId: routeTable.RouteTableId,
}

log.Printf("[DEBUG] Deleting EC2 Route Table (%s) Route", id)
_, err := conn.DeleteRoute(input)

if err != nil {
sweeperErr := fmt.Errorf("error deleting EC2 Route Table (%s) Route: %w", id, err)
log.Printf("[ERROR] %s", sweeperErr)
sweeperErrs = multierror.Append(sweeperErrs, sweeperErr)
continue
}
}

continue
}

input := &ec2.DeleteRouteTableInput{
RouteTableId: routeTable.RouteTableId,
}

log.Printf("[DEBUG] Deleting Route Table: %s", input)
log.Printf("[DEBUG] Deleting EC2 Route Table: %s", id)
_, err := conn.DeleteRouteTable(input)

if err != nil {
log.Printf("[ERROR] Error deleting Route Table (%s): %s", aws.StringValue(routeTable.RouteTableId), err)
sweeperErr := fmt.Errorf("error deleting EC2 Route Table (%s): %w", id, err)
log.Printf("[ERROR] %s", sweeperErr)
sweeperErrs = multierror.Append(sweeperErrs, sweeperErr)
continue
}
}

Expand All @@ -71,14 +132,14 @@ func testSweepRouteTables(region string) error {

if testSweepSkipSweepError(err) {
log.Printf("[WARN] Skipping EC2 Route Table sweep for %s: %s", region, err)
return nil
return sweeperErrs.ErrorOrNil()
}

if err != nil {
return fmt.Errorf("Error describing Route Tables: %s", err)
sweeperErrs = multierror.Append(sweeperErrs, fmt.Errorf("error listing EC2 Route Tables: %w", err))
}

return nil
return sweeperErrs.ErrorOrNil()
}

func TestAccAWSRouteTable_basic(t *testing.T) {
Expand Down
28 changes: 20 additions & 8 deletions aws/resource_aws_vpc_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2"
)

const (
Expand Down Expand Up @@ -358,20 +360,30 @@ func resourceAwsVpcEndpointUpdate(d *schema.ResourceData, meta interface{}) erro
func resourceAwsVpcEndpointDelete(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

log.Printf("[DEBUG] Deleting VPC Endpoint: %s", d.Id())
_, err := conn.DeleteVpcEndpoints(&ec2.DeleteVpcEndpointsInput{
input := &ec2.DeleteVpcEndpointsInput{
VpcEndpointIds: aws.StringSlice([]string{d.Id()}),
})
}

output, err := conn.DeleteVpcEndpoints(input)

if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidVpcEndpointIdNotFound) {
return nil
}

if err != nil {
if isAWSErr(err, "InvalidVpcEndpointId.NotFound", "") {
log.Printf("[DEBUG] VPC Endpoint %s is already gone", d.Id())
} else {
return fmt.Errorf("Error deleting VPC Endpoint: %s", err)
return fmt.Errorf("error deleting EC2 VPC Endpoint (%s): %w", d.Id(), err)
}

if output != nil && len(output.Unsuccessful) > 0 {
err := tfec2.UnsuccessfulItemsError(output.Unsuccessful)

if err != nil {
return fmt.Errorf("error deleting EC2 VPC Endpoint (%s): %w", d.Id(), err)
}
}

if err := vpcEndpointWaitUntilDeleted(conn, d.Id(), d.Timeout(schema.TimeoutDelete)); err != nil {
return fmt.Errorf("error waiting for VPC Endpoint (%s) to delete: %s", d.Id(), err)
return fmt.Errorf("error waiting for EC2 VPC Endpoint (%s) to delete: %w", d.Id(), err)
}

return nil
Expand Down
28 changes: 20 additions & 8 deletions aws/resource_aws_vpc_endpoint_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"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"
tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2"
)

func resourceAwsVpcEndpointService() *schema.Resource {
Expand Down Expand Up @@ -273,20 +275,30 @@ func resourceAwsVpcEndpointServiceUpdate(d *schema.ResourceData, meta interface{
func resourceAwsVpcEndpointServiceDelete(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

log.Printf("[DEBUG] Deleting VPC Endpoint Service: %s", d.Id())
_, err := conn.DeleteVpcEndpointServiceConfigurations(&ec2.DeleteVpcEndpointServiceConfigurationsInput{
input := &ec2.DeleteVpcEndpointServiceConfigurationsInput{
ServiceIds: aws.StringSlice([]string{d.Id()}),
})
}

output, err := conn.DeleteVpcEndpointServiceConfigurations(input)

if tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidVpcEndpointServiceIdNotFound) {
return nil
}

if err != nil {
if isAWSErr(err, "InvalidVpcEndpointServiceId.NotFound", "") {
log.Printf("[DEBUG] VPC Endpoint Service %s is already gone", d.Id())
} else {
return fmt.Errorf("Error deleting VPC Endpoint Service: %s", err.Error())
return fmt.Errorf("error deleting EC2 VPC Endpoint Service (%s): %w", d.Id(), err)
}

if output != nil && len(output.Unsuccessful) > 0 {
err := tfec2.UnsuccessfulItemsError(output.Unsuccessful)

if err != nil {
return fmt.Errorf("error deleting EC2 VPC Endpoint Service (%s): %w", d.Id(), err)
}
}

if err := waitForVpcEndpointServiceDeletion(conn, d.Id()); err != nil {
return fmt.Errorf("Error waiting for VPC Endpoint Service %s to delete: %s", d.Id(), err.Error())
return fmt.Errorf("error waiting for EC2 VPC Endpoint Service (%s) to delete: %w", d.Id(), err)
}

return nil
Expand Down
Loading