Skip to content

Commit

Permalink
Merge pull request #873 from muvaf/fix-my-mess
Browse files Browse the repository at this point in the history
AWS SDK v2 Errors
  • Loading branch information
muvaf authored Oct 18, 2021
2 parents 6f7e613 + 702520b commit 34522c5
Show file tree
Hide file tree
Showing 25 changed files with 98 additions and 687 deletions.
2 changes: 0 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ require (
github.com/aws/aws-sdk-go-v2/credentials v1.4.2
github.com/aws/aws-sdk-go-v2/service/acm v1.6.1
github.com/aws/aws-sdk-go-v2/service/acmpca v1.8.1
github.com/aws/aws-sdk-go-v2/service/cloudformation v1.10.1
github.com/aws/aws-sdk-go-v2/service/ec2 v1.18.0
github.com/aws/aws-sdk-go-v2/service/ecr v1.7.0
github.com/aws/aws-sdk-go-v2/service/eks v1.10.1
Expand All @@ -32,7 +31,6 @@ require (
github.com/mitchellh/copystructure v1.0.0
github.com/onsi/gomega v1.14.0
github.com/pkg/errors v0.9.1
github.com/stretchr/testify v1.7.0
gopkg.in/alecthomas/kingpin.v2 v2.2.6
k8s.io/api v0.21.3
k8s.io/apimachinery v0.21.3
Expand Down
3 changes: 0 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ github.com/aws/aws-sdk-go-v2/service/acm v1.6.1 h1:VtAzCtIBLCwkSdA7L9uG0ZkKeEDSa
github.com/aws/aws-sdk-go-v2/service/acm v1.6.1/go.mod h1:iOP3tLxkXzTlV+BqgIVYmBCGJaZjgDP12WXFopp+Rzw=
github.com/aws/aws-sdk-go-v2/service/acmpca v1.8.1 h1:vTtSRHHKnT7o9DpK9Uxz4HXQ+qg0czIEMh3IZ16edqI=
github.com/aws/aws-sdk-go-v2/service/acmpca v1.8.1/go.mod h1:auQKNheXl81Yu6S5JuTwEjpL+cFPppJgQsp/tS0Du0I=
github.com/aws/aws-sdk-go-v2/service/cloudformation v1.10.1 h1:gzhtXomhFLQ+buBaFDvqmF1zKI4ool1Gbc54d3SHfGM=
github.com/aws/aws-sdk-go-v2/service/cloudformation v1.10.1/go.mod h1:ccHKnr19GgmHUdlpVI8vr36PAfG7Q1V/pT5ZH7Owmq8=
github.com/aws/aws-sdk-go-v2/service/ec2 v1.18.0 h1:5wWtSfYRWgkpKKMW4yJ5llzI9s24Fls7Pv7uw2BiYbk=
github.com/aws/aws-sdk-go-v2/service/ec2 v1.18.0/go.mod h1:d8R2f1hFcknkA3MW4SeExwEua2KpR+dhSrwWlnlwe5Q=
github.com/aws/aws-sdk-go-v2/service/ecr v1.7.0 h1:lgPtXjJNEbRLLlnk8fGDQk80A4cSpPBtyTcI4VSxork=
Expand Down Expand Up @@ -522,7 +520,6 @@ github.com/spf13/viper v1.8.1/go.mod h1:o0Pch8wJ9BVSWGQMbra6iw0oQ5oktSIBaujf1rJH
github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48=
github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
Expand Down
35 changes: 15 additions & 20 deletions pkg/clients/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ import (
credentialsv1 "github.com/aws/aws-sdk-go/aws/credentials"
endpointsv1 "github.com/aws/aws-sdk-go/aws/endpoints"
"github.com/aws/aws-sdk-go/aws/session"
xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
"github.com/crossplane/crossplane-runtime/pkg/resource"
"github.com/aws/smithy-go"
jsonpatch "github.com/evanphx/json-patch"
"github.com/go-ini/ini"
"github.com/google/go-cmp/cmp"
Expand All @@ -45,6 +44,9 @@ import (
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
"github.com/crossplane/crossplane-runtime/pkg/resource"

"github.com/crossplane/provider-aws/apis/v1alpha3"
"github.com/crossplane/provider-aws/apis/v1beta1"
)
Expand Down Expand Up @@ -748,24 +750,17 @@ func IsPolicyUpToDate(local, remote *string) bool {
return cmp.Equal(localUnmarshalled, remoteUnmarshalled, cmpopts.EquateEmpty(), sortSlicesOpt)
}

// CleanError Will remove the requestID from a awserr.Error and return a new awserr.
// If not awserr it will return the original error
func CleanError(err error) error {
// Wrap will remove the request-specific information from the error and only then
// wrap it.
func Wrap(err error, msg string) error {
// NOTE(muvaf): nil check is done for performance, otherwise errors.As makes
// a few reflection calls before returning false, letting awsErr be nil.
if err == nil {
return err
return nil
}
// TODO cvodak revisit this
// var re *awshttp.ResponseError
// if errors.As(err, &re) {
// log.Printf("requestID: %s, error: %v", re.ServiceRequestID(), re.Unwrap());
// }
// if awsErr, ok := err.(awserr.Error); ok {
// return awserr.New(awsErr.Code(), awsErr.Message(), nil)
//}
return err
}

// Wrap Attempts to remove requestID from awserr before calling Wrap
func Wrap(err error, msg string) error {
return errors.Wrap(CleanError(err), msg)
var awsErr smithy.APIError
if errors.As(err, &awsErr) {
return errors.Wrap(awsErr, msg)
}
return errors.Wrap(err, msg)
}
55 changes: 55 additions & 0 deletions pkg/clients/aws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ import (
"fmt"
"testing"

"github.com/aws/aws-sdk-go-v2/aws/transport/http"
"github.com/aws/smithy-go"
smithyhttp "github.com/aws/smithy-go/transport/http"
"github.com/pkg/errors"

"github.com/crossplane/crossplane-runtime/pkg/test"

"github.com/aws/aws-sdk-go-v2/aws"
ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
"github.com/aws/smithy-go/document"
Expand All @@ -31,6 +38,9 @@ import (

const (
awsCredentialsFileFormat = "[%s]\naws_access_key_id = %s\naws_secret_access_key = %s"

errBoom = "boom"
errMsg = "example err msg"
)

func TestCredentialsIdSecret(t *testing.T) {
Expand Down Expand Up @@ -525,3 +535,48 @@ func TestIsPolicyUpToDate(t *testing.T) {
})
}
}

func TestWrap(t *testing.T) {
rootErr := &smithy.GenericAPIError{
Code: "InvalidVpcID.NotFound",
Message: "The vpc ID 'vpc-06f35a4eaed9b4609' does not exist",
Fault: smithy.FaultUnknown,
}
cases := map[string]struct {
reason string
arg error
want error
}{
"Nil": {
arg: nil,
want: nil,
},
"NonAWSError": {
reason: "It should not change anything if the error is not coming from AWS",
arg: errors.New(errBoom),
want: errors.Wrap(errors.New(errBoom), errMsg),
},
"AWSError": {
reason: "Request ID should be removed from the final error if it's an AWS error",
arg: &smithy.OperationError{
ServiceID: "EC2",
OperationName: "DescribeVpcs",
Err: &http.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Err: rootErr,
},
RequestID: "c3dc34d4-b9d6-42a1-9909-7e8f62c6b9cc",
},
},
want: errors.Wrap(rootErr, errMsg),
},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
err := Wrap(tc.arg, errMsg)
if diff := cmp.Diff(tc.want, err, test.EquateErrors()); diff != "" {
t.Errorf("Wrap: -want, +got:\n%s", diff)
}
})
}
}
87 changes: 0 additions & 87 deletions pkg/clients/cloudformation/cloudformation.go

This file was deleted.

43 changes: 0 additions & 43 deletions pkg/clients/cloudformation/fake/fake.go

This file was deleted.

7 changes: 1 addition & 6 deletions pkg/clients/ec2/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,7 @@ type AddressClient interface {
// IsAddressNotFoundErr returns true if the error is because the address doesn't exist
func IsAddressNotFoundErr(err error) bool {
var awsErr smithy.APIError
if errors.As(err, &awsErr) {
if awsErr.ErrorCode() == AddressAddressNotFound || awsErr.ErrorCode() == AddressAllocationNotFound {
return true
}
}
return false
return errors.As(err, &awsErr) && (awsErr.ErrorCode() == AddressAddressNotFound || awsErr.ErrorCode() == AddressAllocationNotFound)
}

// GenerateAddressObservation is used to produce v1beta1.AddressObservation from
Expand Down
8 changes: 1 addition & 7 deletions pkg/clients/ec2/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,7 @@ func NewInstanceClient(cfg aws.Config) InstanceClient {
// IsInstanceNotFoundErr returns true if the error is because the item doesn't exist
func IsInstanceNotFoundErr(err error) bool {
var awsErr smithy.APIError
if errors.As(err, &awsErr) {
if awsErr.ErrorCode() == InstanceNotFound {
return true
}
}

return false
return errors.As(err, &awsErr) && awsErr.ErrorCode() == InstanceNotFound
}

// IsInstanceUpToDate returns true if there is no update-able difference between desired
Expand Down
7 changes: 1 addition & 6 deletions pkg/clients/ec2/internetgateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,7 @@ func NewInternetGatewayClient(cfg aws.Config) InternetGatewayClient {
// IsInternetGatewayNotFoundErr returns true if the error is because the item doesn't exist
func IsInternetGatewayNotFoundErr(err error) bool {
var awsErr smithy.APIError
if errors.As(err, &awsErr) {
if awsErr.ErrorCode() == InternetGatewayIDNotFound {
return true
}
}
return false
return errors.As(err, &awsErr) && awsErr.ErrorCode() == InternetGatewayIDNotFound
}

// IsInternetGatewayAlreadyAttached returns true if the error is because the item doesn't exist
Expand Down
7 changes: 1 addition & 6 deletions pkg/clients/ec2/natgateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,7 @@ func NewNatGatewayClient(cfg aws.Config) NatGatewayClient {
// IsNatGatewayNotFoundErr returns true if the error is because the item doesn't exist
func IsNatGatewayNotFoundErr(err error) bool {
var awsErr smithy.APIError
if errors.As(err, &awsErr) {
if awsErr.ErrorCode() == NatGatewayNotFound {
return true
}
}
return false
return errors.As(err, &awsErr) && awsErr.ErrorCode() == NatGatewayNotFound
}

// GenerateNATGatewayObservation is used to produce v1beta1.NatGatewayObservation from
Expand Down
21 changes: 3 additions & 18 deletions pkg/clients/ec2/routetable.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,34 +54,19 @@ func NewRouteTableClient(cfg aws.Config) RouteTableClient {
// IsRouteTableNotFoundErr returns true if the error is because the route table doesn't exist
func IsRouteTableNotFoundErr(err error) bool {
var awsErr smithy.APIError
if errors.As(err, &awsErr) {
if awsErr.ErrorCode() == RouteTableIDNotFound {
return true
}
}
return false
return errors.As(err, &awsErr) && awsErr.ErrorCode() == RouteTableIDNotFound
}

// IsRouteNotFoundErr returns true if the error is because the route doesn't exist
func IsRouteNotFoundErr(err error) bool {
var awsErr smithy.APIError
if errors.As(err, &awsErr) {
if awsErr.ErrorCode() == RouteNotFound {
return true
}
}
return false
return errors.As(err, &awsErr) && awsErr.ErrorCode() == RouteNotFound
}

// IsAssociationIDNotFoundErr returns true if the error is because the association doesn't exist
func IsAssociationIDNotFoundErr(err error) bool {
var awsErr smithy.APIError
if errors.As(err, &awsErr) {
if awsErr.ErrorCode() == AssociationIDNotFound {
return true
}
}
return false
return errors.As(err, &awsErr) && awsErr.ErrorCode() == AssociationIDNotFound
}

// GenerateRTObservation is used to produce v1beta1.RouteTableExternalStatus from
Expand Down
Loading

0 comments on commit 34522c5

Please sign in to comment.