add smithy error parser

Signed-off-by: Pankaj Walke <[email protected]>

Author: punkwalker

pkg/cloud/awserrors/errors.go

@@ -18,10 +18,13 @@ limitations under the License.
 package awserrors
 
 import (
+	"errors"
 	"net/http"
 
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/ssm"
+	"github.com/aws/smithy-go"
+	smithyhttp "github.com/aws/smithy-go/transport/http"
 )
 
 // Error singletons for AWS errors.
@@ -84,6 +87,13 @@ type EC2Error struct {
 	Code int
 }
 
+// SmithyError holds parsed smithy errors from aws-sdk-go-v2 API calls
+type SmithyError struct {
+	errCode    string
+	errMessage string
+	statusCode int
+}
+
 // Error implements the Error interface.
 func (e *EC2Error) Error() string {
 	return e.msg
@@ -224,3 +234,46 @@ func IsPermissionNotFoundError(err error) bool {
 	}
 	return false
 }
+
+// ParseSmithyError returns parsed SmithyError from AWS API calls.
+func ParseSmithyError(err error) *SmithyError {
+	smithyErr := &SmithyError{}
+
+	if err == nil {
+		return nil
+	}
+
+	var ae smithy.APIError
+	if errors.As(err, &ae) {
+		smithyErr.errCode = ae.ErrorCode()
+		smithyErr.errMessage = ae.Error()
+	}
+
+	var re *smithyhttp.ResponseError
+	if errors.As(err, &re) {
+		if re.Response != nil {
+			smithyErr.statusCode = re.Response.StatusCode
+		}
+		var innerAE smithy.APIError
+		if re.Err != nil && errors.As(re.Err, &innerAE) {
+			smithyErr.errCode = innerAE.ErrorCode()
+		}
+	}
+
+	return smithyErr
+}
+
+// ErrorCode returns the error code from the SmithyError.
+func (s *SmithyError) ErrorCode() string {
+	return s.errCode
+}
+
+// ErrorMessage returns the error message from the SmithyError.
+func (s *SmithyError) ErrorMessage() string {
+	return s.errMessage
+}
+
+// StatusCode returns the status code from the SmithyError.
+func (s *SmithyError) StatusCode() int {
+	return s.statusCode
+}

pkg/cloud/metricsv2/metrics.go

@@ -19,19 +19,18 @@ package metricsv2
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"strconv"
 	"time"
 
 	awsmiddleware "github.com/aws/aws-sdk-go-v2/aws/middleware"
-	"github.com/aws/smithy-go"
 	"github.com/aws/smithy-go/middleware"
 	smithyhttp "github.com/aws/smithy-go/transport/http"
 	"github.com/prometheus/client_golang/prometheus"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/metrics"
 
+	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/awserrors"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/record"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/version"
 )
@@ -165,7 +164,9 @@ func getAttemptContextMiddleware() middleware.FinalizeMiddleware {
 			}
 
 			if err != nil {
-				request.ErrorCode, _, request.StatusCode = parseSmithyError(err)
+				smithyErr := awserrors.ParseSmithyError(err)
+				request.ErrorCode = smithyErr.ErrorCode()
+				request.StatusCode = smithyErr.StatusCode()
 			}
 		}
 
@@ -180,7 +181,8 @@ func getRecordAWSPermissionsIssueMiddleware(target runtime.Object) middleware.Fi
 			request := getContext(ctx)
 			if request != nil {
 				var errMessage string
-				request.ErrorCode, errMessage, _ = parseSmithyError(err)
+				smithyErr := awserrors.ParseSmithyError(err)
+				request.ErrorCode = smithyErr.ErrorCode()
 				switch request.ErrorCode {
 				case "AccessDenied", "AuthFailure", "UnauthorizedOperation", "NoCredentialProviders",
 					"ExpiredToken", "InvalidClientTokenId", "SignatureDoesNotMatch", "ValidationError":
@@ -215,35 +217,6 @@ func getContext(ctx context.Context) *RequestData {
 	return rctx.(*RequestData)
 }
 
-// parseSmithyError parse Smithy Error and returns errorCode, errorMessage and statusCode.
-func parseSmithyError(err error) (string, string, int) {
-	var errCode, errMessage string
-	var statusCode int
-
-	if err == nil {
-		return errCode, errMessage, statusCode
-	}
-
-	var ae smithy.APIError
-	if errors.As(err, &ae) {
-		errCode = ae.ErrorCode()
-		errMessage = ae.Error()
-	}
-
-	var re *smithyhttp.ResponseError
-	if errors.As(err, &re) {
-		if re.Response != nil {
-			statusCode = re.Response.StatusCode
-		}
-		var innerAE smithy.APIError
-		if re.Err != nil && errors.As(re.Err, &innerAE) {
-			errCode = innerAE.ErrorCode()
-		}
-	}
-
-	return errCode, errMessage, statusCode
-}
-
 // CaptureRequestMetrics will monitor and capture request metrics.
 func (r *RequestData) CaptureRequestMetrics() {
 	if r.Service == "" || r.Region == "" || r.OperationName == "" || r.Controller == "" {

pkg/cloud/services/s3/s3.go

@@ -31,12 +31,12 @@ import (
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/sts"
 	"github.com/aws/aws-sdk-go/service/sts/stsiface"
-	"github.com/aws/smithy-go"
 	"github.com/pkg/errors"
 	"k8s.io/utils/ptr"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
 	iam "sigs.k8s.io/cluster-api-provider-aws/v2/iam/api/v1beta1"
+	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/awserrors"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/util/system"
 )
@@ -122,21 +122,15 @@ func (s *Service) DeleteBucket(ctx context.Context) error {
 		Bucket: aws.String(bucketName),
 	})
 	if err != nil {
-		var aerr smithy.APIError
-		if errors.As(err, &aerr) {
-			switch aerr.ErrorCode() {
-			case (&types.NoSuchBucket{}).ErrorCode():
-				log.Info("Bucket already removed")
-			case "BucketNotEmpty":
-				log.Info("Bucket not empty, skipping removal")
-			default:
-				return errors.Wrap(aerr, "deleting S3 bucket")
-			}
-
-			return nil
+		smithyErr := awserrors.ParseSmithyError(err)
+		switch smithyErr.ErrorCode() {
+		case (&types.NoSuchBucket{}).ErrorCode():
+			log.Info("Bucket already removed")
+		case "BucketNotEmpty":
+			log.Info("Bucket not empty, skipping removal")
+		default:
+			return errors.Wrap(err, "deleting S3 bucket")
 		}
-
-		return errors.Wrap(err, "deleting S3 bucket")
 	}
 
 	return nil
@@ -211,32 +205,30 @@ func (s *Service) Delete(ctx context.Context, m *scope.MachineScope) error {
 		Key:    aws.String(key),
 	})
 	if err != nil {
-		var aerr smithy.APIError
-		if errors.As(err, &aerr) {
-			switch aerr.ErrorCode() {
-			case "Forbidden":
-				// In the case that the IAM policy does not have sufficient
-				// permissions to get the object, we will attempt to delete it
-				// anyway for backwards compatibility reasons.
-				s.scope.Debug("Received 403 forbidden from S3 HeadObject call. If GetObject permission has been granted to the controller but not ListBucket, object is already deleted. Attempting deletion anyway in case GetObject permission hasn't been granted to the controller but DeleteObject has.", "bucket", bucket, "key", key)
-
-				if err := s.deleteObject(ctx, bucket, key); err != nil {
-					return err
-				}
+		smithyErr := awserrors.ParseSmithyError(err)
+		switch smithyErr.ErrorCode() {
+		case "Forbidden":
+			// In the case that the IAM policy does not have sufficient
+			// permissions to get the object, we will attempt to delete it
+			// anyway for backwards compatibility reasons.
+			s.scope.Debug("Received 403 forbidden from S3 HeadObject call. If GetObject permission has been granted to the controller but not ListBucket, object is already deleted. Attempting deletion anyway in case GetObject permission hasn't been granted to the controller but DeleteObject has.", "bucket", bucket, "key", key)
+
+			if err := s.deleteObject(ctx, bucket, key); err != nil {
+				return err
+			}
 
-				s.scope.Debug("Delete object call succeeded despite missing GetObject permission", "bucket", bucket, "key", key)
+			s.scope.Debug("Delete object call succeeded despite missing GetObject permission", "bucket", bucket, "key", key)
 
-				return nil
-			case "NotFound":
-				s.scope.Debug("Either bucket or object does not exist", "bucket", bucket, "key", key)
-				return nil
-			case (&types.NoSuchKey{}).ErrorCode():
-				s.scope.Debug("Object already deleted", "bucket", bucket, "key", key)
-				return nil
-			case (&types.NoSuchBucket{}).ErrorCode():
-				s.scope.Debug("Bucket does not exist", "bucket", bucket)
-				return nil
-			}
+			return nil
+		case "NotFound":
+			s.scope.Debug("Either bucket or object does not exist", "bucket", bucket, "key", key)
+			return nil
+		case (&types.NoSuchKey{}).ErrorCode():
+			s.scope.Debug("Object already deleted", "bucket", bucket, "key", key)
+			return nil
+		case (&types.NoSuchBucket{}).ErrorCode():
+			s.scope.Debug("Bucket does not exist", "bucket", bucket)
+			return nil
 		}
 		return errors.Wrap(err, "deleting S3 object")
 	}
@@ -252,13 +244,11 @@ func (s *Service) deleteObject(ctx context.Context, bucket, key string) error {
 		Key:    aws.String(key),
 	}); err != nil {
 		if ptr.Deref(s.scope.Bucket().BestEffortDeleteObjects, false) {
-			var aerr smithy.APIError
-			if errors.As(err, &aerr) {
-				switch aerr.ErrorCode() {
-				case "Forbidden", "AccessDenied":
-					s.scope.Debug("Ignoring deletion error", "bucket", bucket, "key", key, "error", aerr.ErrorMessage())
-					return nil
-				}
+			smithyErr := awserrors.ParseSmithyError(err)
+			switch smithyErr.ErrorCode() {
+			case "Forbidden", "AccessDenied":
+				s.scope.Debug("Ignoring deletion error", "bucket", bucket, "key", key, "error", smithyErr.ErrorMessage())
+				return nil
 			}
 		}
 		return errors.Wrap(err, "deleting S3 object")
@@ -284,20 +274,16 @@ func (s *Service) createBucketIfNotExist(ctx context.Context, bucketName string)
 		return nil
 	}
 
-	var aerr smithy.APIError
-	if errors.As(err, &aerr) {
-		switch aerr.ErrorCode() {
-		// If bucket already exists, all good.
-		//
-		// TODO: This will fail if bucket is shared with other cluster.
-		case (&types.BucketAlreadyOwnedByYou{}).ErrorCode():
-			return nil
-		default:
-			return errors.Wrap(aerr, "creating S3 bucket")
-		}
+	smithyErr := awserrors.ParseSmithyError(err)
+	switch smithyErr.ErrorCode() {
+	// If bucket already exists, all good.
+	//
+	// TODO: This will fail if bucket is shared with other cluster.
+	case (&types.BucketAlreadyOwnedByYou{}).ErrorCode():
+		return nil
+	default:
+		return errors.Wrap(err, "creating S3 bucket")
 	}
-
-	return errors.Wrap(err, "creating S3 bucket")
 }
 
 func (s *Service) ensureBucketPolicy(ctx context.Context, bucketName string) error {