From a0cb3b8fee5420152b0f9988db5945ba917c6a50 Mon Sep 17 00:00:00 2001 From: Damian Debkowski Date: Mon, 25 Mar 2024 21:16:15 -0700 Subject: [PATCH] feat: parse s3 api errors --- plugin/service/storage/error.go | 104 +++++++++++ plugin/service/storage/error_test.go | 148 +++++++++++++++ plugin/service/storage/plugin.go | 39 ++-- plugin/service/storage/plugin_test.go | 257 +++++++++++++++++--------- plugin/service/storage/testing.go | 30 ++- 5 files changed, 448 insertions(+), 130 deletions(-) create mode 100644 plugin/service/storage/error.go create mode 100644 plugin/service/storage/error_test.go diff --git a/plugin/service/storage/error.go b/plugin/service/storage/error.go new file mode 100644 index 0000000..d502ecd --- /dev/null +++ b/plugin/service/storage/error.go @@ -0,0 +1,104 @@ +package storage + +import ( + stderr "errors" + "fmt" + + "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/aws/smithy-go" + "github.com/hashicorp/boundary-plugin-aws/internal/errors" + pb "github.com/hashicorp/boundary/sdk/pbs/plugin" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +const ( + s3StatusErrMsg = "aws s3 error: %s" + + // s3ErrorBadDigest is returned when there is a checksum + // mismatch between the request and the s3 service. This + // error code is returned for the PutObject request. + s3ErrorBadDigest = "BadDigest" +) + +// parseS3Error will convert an S3 api error into a RPC status. If the +// error does not match against an s3 error type, it will fallback to +// evaluating against generic aws service errors. This includes throttling, +// credentials, and connectivity error types. +// +// https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html +func parseS3Error(err error, msg string) (st *status.Status) { + if err == nil { + return nil + } + + var plgErr *pb.PluginError + defer func() { + if plgErr != nil { + st, err = st.WithDetails(plgErr) + if err != nil { + st = status.New(codes.Internal, err.Error()) + } + } + }() + + // The NoSuchBucket error is returned when trying to access + // an s3 bucket that does not exist. + var missingStore *types.NoSuchBucket + if stderr.As(err, &missingStore) { + plgErr = &pb.PluginError{ + Code: pb.ERROR_ERROR_STORAGE_NO_SUCH_BUCKET, + Message: missingStore.ErrorMessage(), + Nonretryable: true, + } + return status.New(codes.NotFound, fmt.Sprintf(s3StatusErrMsg, msg)) + } + + // The NoSuchKey error is returned when trying to download + // an object from s3 that does not exist. + var missingObject *types.NoSuchKey + if stderr.As(err, &missingObject) { + plgErr = &pb.PluginError{ + Code: pb.ERROR_ERROR_STORAGE_NO_SUCH_OBJECT, + Message: missingObject.ErrorMessage(), + Nonretryable: true, + } + return status.New(codes.NotFound, fmt.Sprintf(s3StatusErrMsg, msg)) + } + + var notFound *types.NotFound + if stderr.As(err, ¬Found) { + plgErr = &pb.PluginError{ + Code: pb.ERROR_ERROR_STORAGE_NO_SUCH_OBJECT, + Message: notFound.ErrorMessage(), + Nonretryable: true, + } + return status.New(codes.NotFound, fmt.Sprintf(s3StatusErrMsg, msg)) + } + + // The InvalidObjectState error is returned when trying to + // access an object that was moved into cold storage. + var invalidState *types.InvalidObjectState + if stderr.As(err, &invalidState) { + plgErr = &pb.PluginError{ + Code: pb.ERROR_ERROR_STORAGE_NO_SUCH_OBJECT, + Message: invalidState.ErrorMessage(), + Nonretryable: true, + } + return status.New(codes.NotFound, fmt.Sprintf(s3StatusErrMsg, msg)) + } + + var apiErr smithy.APIError + if stderr.As(err, &apiErr) { + if apiErr.ErrorCode() == s3ErrorBadDigest { + plgErr = &pb.PluginError{ + Code: pb.ERROR_ERROR_STORAGE_CHECKSUM_MISMATCH, + Message: apiErr.ErrorMessage(), + Nonretryable: false, + } + return status.New(codes.Aborted, fmt.Sprintf(s3StatusErrMsg, msg)) + } + } + + return errors.ParseAWSError(err, msg) +} diff --git a/plugin/service/storage/error_test.go b/plugin/service/storage/error_test.go new file mode 100644 index 0000000..e9a2b0b --- /dev/null +++ b/plugin/service/storage/error_test.go @@ -0,0 +1,148 @@ +package storage + +import ( + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/hashicorp/boundary-plugin-aws/internal/errors" + pb "github.com/hashicorp/boundary/sdk/pbs/plugin" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" +) + +func Test_ParseS3Error(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + err error + expectedStatusCode codes.Code + expectedStatusMsg string + expectedPluginError *pb.PluginError + }{ + { + name: "nil-error", + expectedStatusCode: codes.OK, + }, + { + name: "no-such-bucket", + err: &types.NoSuchBucket{ + Message: aws.String("no such bucket"), + }, + expectedStatusCode: codes.NotFound, + expectedStatusMsg: "aws s3 error: test", + expectedPluginError: &pb.PluginError{ + Code: pb.ERROR_ERROR_STORAGE_NO_SUCH_BUCKET, + Message: "no such bucket", + Nonretryable: true, + }, + }, + { + name: "no-such-key", + err: &types.NoSuchKey{ + Message: aws.String("no such key"), + }, + expectedStatusCode: codes.NotFound, + expectedStatusMsg: "aws s3 error: test", + expectedPluginError: &pb.PluginError{ + Code: pb.ERROR_ERROR_STORAGE_NO_SUCH_OBJECT, + Message: "no such key", + Nonretryable: true, + }, + }, + { + name: "not-found", + err: &types.NotFound{ + Message: aws.String("not found"), + }, + expectedStatusCode: codes.NotFound, + expectedStatusMsg: "aws s3 error: test", + expectedPluginError: &pb.PluginError{ + Code: pb.ERROR_ERROR_STORAGE_NO_SUCH_OBJECT, + Message: "not found", + Nonretryable: true, + }, + }, + { + name: "invalid-object-state", + err: &types.InvalidObjectState{ + Message: aws.String("invalid object state"), + }, + expectedStatusCode: codes.NotFound, + expectedStatusMsg: "aws s3 error: test", + expectedPluginError: &pb.PluginError{ + Code: pb.ERROR_ERROR_STORAGE_NO_SUCH_OBJECT, + Message: "invalid object state", + Nonretryable: true, + }, + }, + { + name: "bad-digest", + err: errors.TestAwsError(s3ErrorBadDigest, "checksum mismatch"), + expectedStatusCode: codes.Aborted, + expectedStatusMsg: "aws s3 error: test", + expectedPluginError: &pb.PluginError{ + Code: pb.ERROR_ERROR_STORAGE_CHECKSUM_MISMATCH, + Message: "checksum mismatch", + Nonretryable: false, + }, + }, + { + name: "fallback-throttle-error", + err: TestAwsS3Error("Throttling", "throttling exception"), + expectedStatusCode: codes.Unavailable, + expectedStatusMsg: "aws service s3: throttling error: test", + expectedPluginError: &pb.PluginError{ + Code: pb.ERROR_ERROR_THROTTLING, + Message: "throttling exception", + Nonretryable: false, + }, + }, + { + name: "fallback-connectivity-error", + err: TestAwsS3Error("RequestTimeout", "request timeout"), + expectedStatusCode: codes.DeadlineExceeded, + expectedStatusMsg: "aws service s3: connectivity error: test", + expectedPluginError: &pb.PluginError{ + Code: pb.ERROR_ERROR_TIMEOUT, + Message: "request timeout", + Nonretryable: false, + }, + }, + { + name: "fallback-credentials-error", + err: TestAwsS3Error("AccessDenied", "access denied"), + expectedStatusCode: codes.PermissionDenied, + expectedStatusMsg: "aws service s3: invalid credentials error: test", + expectedPluginError: &pb.PluginError{ + Code: pb.ERROR_ERROR_INVALID_CREDENTIAL, + Message: "access denied", + Nonretryable: true, + }, + }, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + + actualStatus := parseS3Error(tt.err, "test") + if tt.expectedStatusCode == codes.OK { + require.Nil(actualStatus) + return + } + + require.Equal(tt.expectedStatusCode, actualStatus.Code()) + require.Equal(tt.expectedStatusMsg, actualStatus.Message()) + require.Len(actualStatus.Details(), 1) + for _, detail := range actualStatus.Details() { + switch errDetail := detail.(type) { + case *pb.PluginError: + require.Equal(tt.expectedPluginError.Code, errDetail.Code) + require.Contains(errDetail.Message, tt.expectedPluginError.Message) + require.Equal(tt.expectedPluginError.Nonretryable, errDetail.Nonretryable) + } + } + }) + } +} diff --git a/plugin/service/storage/plugin.go b/plugin/service/storage/plugin.go index c258348..5052856 100644 --- a/plugin/service/storage/plugin.go +++ b/plugin/service/storage/plugin.go @@ -15,7 +15,6 @@ import ( "strings" "github.com/aws/aws-sdk-go-v2/aws" - "github.com/aws/aws-sdk-go-v2/aws/retry" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/google/uuid" @@ -181,7 +180,7 @@ func (p *StoragePlugin) OnUpdateStorageBucket(ctx context.Context, req *pb.OnUpd return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err) } if err := dryRunValidation(ctx, newStorageState, newStorageAttributes, newBucket); err != nil { - return nil, status.Errorf(codes.InvalidArgument, "failed to validate incoming credentials: %s", err) + return nil, err } // Replace the existing credential state. @@ -355,7 +354,7 @@ func (p *StoragePlugin) HeadObject(ctx context.Context, req *pb.HeadObjectReques Key: aws.String(objectKey), }) if err != nil { - return nil, parseAWSErrCode("error getting head object from s3", err) + return nil, parseS3Error(err, "failed to head object").Err() } return &pb.HeadObjectResponse{ ContentLength: resp.ContentLength, @@ -481,7 +480,7 @@ func (p *StoragePlugin) GetObject(req *pb.GetObjectRequest, stream pb.StoragePlu Key: aws.String(objectKey), }) if err != nil { - return parseAWSErrCode("error getting object from s3", err) + return parseS3Error(err, "failed to get object").Err() } defer resp.Body.Close() @@ -606,7 +605,7 @@ func (p *StoragePlugin) PutObject(ctx context.Context, req *pb.PutObjectRequest) ChecksumSHA256: aws.String(checksum), }) if err != nil { - return nil, parseAWSErrCode("error putting object into s3", err) + return nil, parseS3Error(err, "failed to put object").Err() } if resp.ChecksumSHA256 == nil { return nil, status.Errorf(codes.Internal, "missing checksum response from aws") @@ -694,7 +693,7 @@ func (p *StoragePlugin) DeleteObjects(ctx context.Context, req *pb.DeleteObjects Key: aws.String(prefix), }) if err != nil { - return nil, parseAWSErrCode("error deleting S3 object", err) + return nil, parseS3Error(err, "failed to delete object").Err() } return &pb.DeleteObjectsResponse{ ObjectsDeleted: uint32(1), @@ -714,7 +713,7 @@ func (p *StoragePlugin) DeleteObjects(ctx context.Context, req *pb.DeleteObjects ContinuationToken: conToken, }) if err != nil { - return nil, parseAWSErrCode("error iterating S3 bucket contents", err) + return nil, parseS3Error(err, "failed to list objects").Err() } truncated = res.IsTruncated conToken = res.NextContinuationToken @@ -736,7 +735,7 @@ func (p *StoragePlugin) DeleteObjects(ctx context.Context, req *pb.DeleteObjects }, }) if err != nil { - return nil, parseAWSErrCode("error deleting S3 object(s)", err) + return nil, parseS3Error(err, "failed to delete objects").Err() } deleted += len(res.Deleted) } @@ -779,28 +778,28 @@ func dryRunValidation(ctx context.Context, state *awsStoragePersistedState, attr Key: aws.String(objectKey), Body: bytes.NewReader([]byte("hashicorp boundary aws plugin access test")), }); err != nil { - return parseAWSErrCode("error failed to put object", err) + return parseS3Error(err, "failed to put object").Err() } if _, err := client.GetObject(ctx, &s3.GetObjectInput{ Bucket: aws.String(bucket.GetBucketName()), Key: aws.String(objectKey), }); err != nil { - return parseAWSErrCode("error failed to get object", err) + return parseS3Error(err, "failed to get object").Err() } if _, err := client.HeadObject(ctx, &s3.HeadObjectInput{ Bucket: aws.String(bucket.GetBucketName()), Key: aws.String(objectKey), }); err != nil { - return parseAWSErrCode("error failed to get head object", err) + return parseS3Error(err, "failed to head object").Err() } if res, err := client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{ Bucket: aws.String(bucket.GetBucketName()), Prefix: aws.String(objectKey), }); err != nil { - return parseAWSErrCode("error failed to list objects", err) + return parseS3Error(err, "failed to list objects").Err() } else if res == nil || len(res.Contents) != 1 || *res.Contents[0].Key != objectKey { return status.Errorf(codes.InvalidArgument, "list response did not contain the expected key: %+v", res) } @@ -809,22 +808,8 @@ func dryRunValidation(ctx context.Context, state *awsStoragePersistedState, attr Bucket: aws.String(bucket.GetBucketName()), Key: aws.String(objectKey), }); err != nil { - return parseAWSErrCode("error failed to delete object", err) + return parseS3Error(err, "failed to delete object").Err() } return nil } - -// parseAWSErrCode detects if an aws error is a throttle error. -// If the error is a throttle error, then an Unavailable status code -// is returned. Otherwise the default status code is an Internal error. -func parseAWSErrCode(msg string, err error) error { - errCode := codes.Internal - retryErr := retry.ThrottleErrorCode{ - Codes: retry.DefaultThrottleErrorCodes, - } - if retryErr.IsErrorThrottle(err).Bool() { - errCode = codes.Unavailable - } - return status.Errorf(errCode, "%s: %s", msg, err) -} diff --git a/plugin/service/storage/plugin_test.go b/plugin/service/storage/plugin_test.go index 5e07ca4..fd9dfce 100644 --- a/plugin/service/storage/plugin_test.go +++ b/plugin/service/storage/plugin_test.go @@ -153,11 +153,11 @@ func TestStoragePlugin_OnCreateStorageBucket(t *testing.T) { storageOpts: []awsStoragePersistedStateOption{ withTestS3APIFunc(newTestMockS3( nil, - testMockS3WithPutObjectError(errors.New(testPutObjectErr)), + testMockS3WithPutObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform PutObject action")), )), }, - expectedErrContains: "error failed to put object: test error for PutObject", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to put object", + expectedErrCode: codes.PermissionDenied, }, { name: "dryRunValidation putObject throttle", @@ -166,10 +166,10 @@ func TestStoragePlugin_OnCreateStorageBucket(t *testing.T) { storageOpts: []awsStoragePersistedStateOption{ withTestS3APIFunc(newTestMockS3( nil, - testMockS3WithPutObjectError(new(throttleErr)), + testMockS3WithPutObjectError(TestAwsS3Error("ThrottlingException", "throttling exception")), )), }, - expectedErrContains: "error failed to put object: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to put object", expectedErrCode: codes.Unavailable, }, { @@ -181,12 +181,12 @@ func TestStoragePlugin_OnCreateStorageBucket(t *testing.T) { newTestMockS3( nil, testMockS3WithPutObjectOutput(&s3.PutObjectOutput{}), - testMockS3WithGetObjectError(errors.New(testGetObjectErr)), + testMockS3WithGetObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform GetObject action")), ), ), }, - expectedErrContains: "error failed to get object: test error for GetObject", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to get object", + expectedErrCode: codes.PermissionDenied, }, { name: "dryRunValidation failed getObject throttle", @@ -197,11 +197,11 @@ func TestStoragePlugin_OnCreateStorageBucket(t *testing.T) { newTestMockS3( nil, testMockS3WithPutObjectOutput(&s3.PutObjectOutput{}), - testMockS3WithGetObjectError(new(throttleErr)), + testMockS3WithGetObjectError(TestAwsS3Error("ThrottlingException", "throttling exception")), ), ), }, - expectedErrContains: "error failed to get object: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to get object", expectedErrCode: codes.Unavailable, }, { @@ -214,12 +214,12 @@ func TestStoragePlugin_OnCreateStorageBucket(t *testing.T) { nil, testMockS3WithPutObjectOutput(&s3.PutObjectOutput{}), testMockS3WithGetObjectOutput(&s3.GetObjectOutput{}), - testMockS3WithHeadObjectError(errors.New(testHeadObjectErr)), + testMockS3WithHeadObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform HeadObject action")), ), ), }, - expectedErrContains: "error failed to get head object: test error for HeadObject", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to head object", + expectedErrCode: codes.PermissionDenied, }, { name: "dryRunValidation failed headObject throttle", @@ -231,11 +231,11 @@ func TestStoragePlugin_OnCreateStorageBucket(t *testing.T) { nil, testMockS3WithPutObjectOutput(&s3.PutObjectOutput{}), testMockS3WithGetObjectOutput(&s3.GetObjectOutput{}), - testMockS3WithHeadObjectError(new(throttleErr)), + testMockS3WithHeadObjectError(TestAwsS3Error("ThrottlingException", "throttling exception")), ), ), }, - expectedErrContains: "error failed to get head object: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to head object", expectedErrCode: codes.Unavailable, }, { @@ -731,11 +731,11 @@ func TestStoragePlugin_OnUpdateStorageBucket(t *testing.T) { storageOpts: []awsStoragePersistedStateOption{ withTestS3APIFunc(newTestMockS3( nil, - testMockS3WithPutObjectError(errors.New(testPutObjectErr)), + testMockS3WithPutObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform PutObject action")), )), }, - expectedErrContains: "error failed to put object: test error for PutObject", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to put object", + expectedErrCode: codes.PermissionDenied, }, { name: "dryRunValidation failed putObject throttle", @@ -744,10 +744,10 @@ func TestStoragePlugin_OnUpdateStorageBucket(t *testing.T) { storageOpts: []awsStoragePersistedStateOption{ withTestS3APIFunc(newTestMockS3( nil, - testMockS3WithPutObjectError(new(throttleErr)), + testMockS3WithPutObjectError(TestAwsS3Error("ThrottlingException", "throttling exception")), )), }, - expectedErrContains: "error failed to put object: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to put object", expectedErrCode: codes.Unavailable, }, { @@ -759,12 +759,12 @@ func TestStoragePlugin_OnUpdateStorageBucket(t *testing.T) { newTestMockS3( nil, testMockS3WithPutObjectOutput(&s3.PutObjectOutput{}), - testMockS3WithGetObjectError(errors.New(testGetObjectErr)), + testMockS3WithGetObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform GetObject action")), ), ), }, - expectedErrContains: "error failed to get object: test error for GetObject", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to get object", + expectedErrCode: codes.PermissionDenied, }, { name: "dryRunValidation failed getObject throttle", @@ -775,11 +775,11 @@ func TestStoragePlugin_OnUpdateStorageBucket(t *testing.T) { newTestMockS3( nil, testMockS3WithPutObjectOutput(&s3.PutObjectOutput{}), - testMockS3WithGetObjectError(new(throttleErr)), + testMockS3WithGetObjectError(TestAwsS3Error("ThrottlingException", "throttling exception")), ), ), }, - expectedErrContains: "error failed to get object: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to get object", expectedErrCode: codes.Unavailable, }, { @@ -792,12 +792,12 @@ func TestStoragePlugin_OnUpdateStorageBucket(t *testing.T) { nil, testMockS3WithPutObjectOutput(&s3.PutObjectOutput{}), testMockS3WithGetObjectOutput(&s3.GetObjectOutput{}), - testMockS3WithHeadObjectError(errors.New(testHeadObjectErr)), + testMockS3WithHeadObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform HeadObject action")), ), ), }, - expectedErrContains: "error failed to get head object: test error for HeadObject", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to head object", + expectedErrCode: codes.PermissionDenied, }, { name: "dryRunValidation failed headObject throttle", @@ -809,11 +809,11 @@ func TestStoragePlugin_OnUpdateStorageBucket(t *testing.T) { nil, testMockS3WithPutObjectOutput(&s3.PutObjectOutput{}), testMockS3WithGetObjectOutput(&s3.GetObjectOutput{}), - testMockS3WithHeadObjectError(new(throttleErr)), + testMockS3WithHeadObjectError(TestAwsS3Error("ThrottlingException", "throttling exception")), ), ), }, - expectedErrContains: "error failed to get head object: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to head object", expectedErrCode: codes.Unavailable, }, { @@ -954,14 +954,14 @@ func TestStoragePlugin_OnUpdateStorageBucket(t *testing.T) { withTestS3APIFunc( newTestMockS3( nil, - testMockS3WithHeadObjectError(fmt.Errorf("head object failed oops")), + testMockS3WithHeadObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform HeadObject action")), testMockS3WithGetObjectOutput(&s3.GetObjectOutput{}), testMockS3WithPutObjectOutput(&s3.PutObjectOutput{}), ), ), }, - expectedErrContains: "head object failed oops", - expectedErrCode: codes.InvalidArgument, + expectedErrContains: "aws service s3: invalid credentials error: failed to head object", + expectedErrCode: codes.PermissionDenied, }, { name: "dynamic to dynamic credentials success", @@ -1044,14 +1044,14 @@ func TestStoragePlugin_OnUpdateStorageBucket(t *testing.T) { withTestS3APIFunc( newTestMockS3( nil, - testMockS3WithPutObjectError(fmt.Errorf("put object failed oops")), + testMockS3WithPutObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform PutObject action")), testMockS3WithGetObjectOutput(&s3.GetObjectOutput{}), testMockS3WithHeadObjectOutput(&s3.HeadObjectOutput{}), ), ), }, - expectedErrContains: "put object failed oops", - expectedErrCode: codes.InvalidArgument, + expectedErrContains: "aws service s3: invalid credentials error: failed to put object", + expectedErrCode: codes.PermissionDenied, }, { name: "success static non rotated to dynamic credentials", @@ -1262,7 +1262,7 @@ func TestStoragePlugin_OnUpdateStorageBucket(t *testing.T) { withTestS3APIFunc( newTestMockS3( nil, - testMockS3WithPutObjectError(fmt.Errorf("put object fail oops")), + testMockS3WithPutObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform PutObject action")), testMockS3WithGetObjectOutput(&s3.GetObjectOutput{}), testMockS3WithHeadObjectOutput(&s3.HeadObjectOutput{}), testMockS3WithListObjectsV2OutputFunc(func(i *s3.ListObjectsV2Input) *s3.ListObjectsV2Output { @@ -1277,8 +1277,8 @@ func TestStoragePlugin_OnUpdateStorageBucket(t *testing.T) { ), ), }, - expectedErrContains: "put object fail oops", - expectedErrCode: codes.InvalidArgument, + expectedErrContains: "aws service s3: invalid credentials error: failed to put object", + expectedErrCode: codes.PermissionDenied, }, { name: "success dynamic to static non-rotated credentials", @@ -1509,7 +1509,7 @@ func TestStoragePlugin_OnUpdateStorageBucket(t *testing.T) { if tc.expectedErrContains != "" { require.NotNil(err) require.Contains(err.Error(), tc.expectedErrContains) - require.Equal(status.Code(err).String(), tc.expectedErrCode.String()) + require.Equal(tc.expectedErrCode.String(), status.Code(err).String()) return } require.NoError(err) @@ -1799,12 +1799,12 @@ func TestStoragePlugin_HeadObject(t *testing.T) { withTestS3APIFunc( newTestMockS3( nil, - testMockS3WithHeadObjectError(errors.New(testHeadObjectErr)), + testMockS3WithHeadObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform HeadObject action")), ), ), }, - expectedErrContains: "error getting head object from s3: test error for HeadObject", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to head object", + expectedErrCode: codes.PermissionDenied, }, { name: "headObject throttle error", @@ -1814,13 +1814,30 @@ func TestStoragePlugin_HeadObject(t *testing.T) { withTestS3APIFunc( newTestMockS3( nil, - testMockS3WithHeadObjectError(new(throttleErr)), + testMockS3WithHeadObjectError(TestAwsS3Error("ThrottlingException", "throttling exception")), ), ), }, - expectedErrContains: "error getting head object from s3: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to head object", expectedErrCode: codes.Unavailable, }, + { + name: "headObject not found error", + req: validRequest(), + credOpts: validSTSMock(), + storageOpts: []awsStoragePersistedStateOption{ + withTestS3APIFunc( + newTestMockS3( + nil, + testMockS3WithHeadObjectError(&s3types.NotFound{ + Message: aws.String("resource not found"), + }), + ), + ), + }, + expectedErrContains: "aws s3 error: failed to head object", + expectedErrCode: codes.NotFound, + }, { name: "success", req: validRequest(), @@ -1984,11 +2001,11 @@ func TestStoragePlugin_ValidatePermissions(t *testing.T) { storageOpts: []awsStoragePersistedStateOption{ withTestS3APIFunc(newTestMockS3( nil, - testMockS3WithPutObjectError(errors.New(testPutObjectErr)), + testMockS3WithPutObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform PutObject action")), )), }, - expectedErrContains: "error failed to put object: test error for PutObject", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to put object", + expectedErrCode: codes.PermissionDenied, }, { name: "dryRunValidation failed putObject throttle", @@ -1997,10 +2014,10 @@ func TestStoragePlugin_ValidatePermissions(t *testing.T) { storageOpts: []awsStoragePersistedStateOption{ withTestS3APIFunc(newTestMockS3( nil, - testMockS3WithPutObjectError(new(throttleErr)), + testMockS3WithPutObjectError(TestAwsS3Error("ThrottlingException", "throttling exception")), )), }, - expectedErrContains: "error failed to put object: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to put object", expectedErrCode: codes.Unavailable, }, { @@ -2012,12 +2029,12 @@ func TestStoragePlugin_ValidatePermissions(t *testing.T) { newTestMockS3( nil, testMockS3WithPutObjectOutput(&s3.PutObjectOutput{}), - testMockS3WithGetObjectError(errors.New(testGetObjectErr)), + testMockS3WithGetObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform GetObject action")), ), ), }, - expectedErrContains: "error failed to get object: test error for GetObject", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to get object", + expectedErrCode: codes.PermissionDenied, }, { name: "dryRunValidation failed getObject throttle", @@ -2028,11 +2045,11 @@ func TestStoragePlugin_ValidatePermissions(t *testing.T) { newTestMockS3( nil, testMockS3WithPutObjectOutput(&s3.PutObjectOutput{}), - testMockS3WithGetObjectError(new(throttleErr)), + testMockS3WithGetObjectError(TestAwsS3Error("ThrottlingException", "throttling exception")), ), ), }, - expectedErrContains: "error failed to get object: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to get object", expectedErrCode: codes.Unavailable, }, { @@ -2045,12 +2062,12 @@ func TestStoragePlugin_ValidatePermissions(t *testing.T) { nil, testMockS3WithPutObjectOutput(&s3.PutObjectOutput{}), testMockS3WithGetObjectOutput(&s3.GetObjectOutput{}), - testMockS3WithHeadObjectError(errors.New(testHeadObjectErr)), + testMockS3WithHeadObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform HeadObject action")), ), ), }, - expectedErrContains: "error failed to get head object: test error for HeadObject", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to head object", + expectedErrCode: codes.PermissionDenied, }, { name: "dryRunValidation failed headObject throttle", @@ -2062,11 +2079,11 @@ func TestStoragePlugin_ValidatePermissions(t *testing.T) { nil, testMockS3WithPutObjectOutput(&s3.PutObjectOutput{}), testMockS3WithGetObjectOutput(&s3.GetObjectOutput{}), - testMockS3WithHeadObjectError(new(throttleErr)), + testMockS3WithHeadObjectError(TestAwsS3Error("ThrottlingException", "throttling exception")), ), ), }, - expectedErrContains: "error failed to get head object: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to head object", expectedErrCode: codes.Unavailable, }, { @@ -2254,12 +2271,63 @@ func TestStoragePlugin_GetObject(t *testing.T) { withTestS3APIFunc( newTestMockS3( nil, - testMockS3WithGetObjectError(errors.New(testGetObjectErr)), + testMockS3WithGetObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform HeadObject action")), ), ), }, - expectedErrContains: "error getting object from s3: test error for GetObject", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to get object", + expectedErrCode: codes.PermissionDenied, + }, + { + name: "getObject no such key", + req: validRequest(), + credOpts: validSTSMock(), + storageOpts: []awsStoragePersistedStateOption{ + withTestS3APIFunc( + newTestMockS3( + nil, + testMockS3WithGetObjectError(&s3types.NoSuchKey{ + Message: aws.String("resource does not exist"), + }), + ), + ), + }, + expectedErrContains: "aws s3 error: failed to get object", + expectedErrCode: codes.NotFound, + }, + { + name: "getObject invalid state", + req: validRequest(), + credOpts: validSTSMock(), + storageOpts: []awsStoragePersistedStateOption{ + withTestS3APIFunc( + newTestMockS3( + nil, + testMockS3WithGetObjectError(&s3types.InvalidObjectState{ + Message: aws.String("resource is in cold storage"), + }), + ), + ), + }, + expectedErrContains: "aws s3 error: failed to get object", + expectedErrCode: codes.NotFound, + }, + { + name: "getObject no such bucket", + req: validRequest(), + credOpts: validSTSMock(), + storageOpts: []awsStoragePersistedStateOption{ + withTestS3APIFunc( + newTestMockS3( + nil, + testMockS3WithGetObjectError(&s3types.NoSuchBucket{ + Message: aws.String("bucket does not exist"), + }), + ), + ), + }, + expectedErrContains: "aws s3 error: failed to get object", + expectedErrCode: codes.NotFound, }, { name: "getObject throttle error", @@ -2269,11 +2337,11 @@ func TestStoragePlugin_GetObject(t *testing.T) { withTestS3APIFunc( newTestMockS3( nil, - testMockS3WithGetObjectError(new(throttleErr)), + testMockS3WithGetObjectError(TestAwsS3Error("ThrottlingException", "throttling exception")), ), ), }, - expectedErrContains: "error getting object from s3: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to get object", expectedErrCode: codes.Unavailable, }, { @@ -2596,12 +2664,29 @@ func TestStoragePlugin_PutObject(t *testing.T) { withTestS3APIFunc( newTestMockS3( nil, - testMockS3WithPutObjectError(errors.New(testPutObjectErr)), + testMockS3WithPutObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform HeadObject action")), ), ), }, - expectedErrContains: "error putting object into s3: test error for PutObject", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to put object", + expectedErrCode: codes.PermissionDenied, + }, + { + name: "putObject no such bucket", + request: validRequest(), + credOpts: validSTSMock(), + storageOpts: []awsStoragePersistedStateOption{ + withTestS3APIFunc( + newTestMockS3( + nil, + testMockS3WithPutObjectError(&s3types.NoSuchBucket{ + Message: aws.String("bucket does not exist"), + }), + ), + ), + }, + expectedErrContains: "aws s3 error: failed to put object", + expectedErrCode: codes.NotFound, }, { name: "throttle error", @@ -2611,11 +2696,11 @@ func TestStoragePlugin_PutObject(t *testing.T) { withTestS3APIFunc( newTestMockS3( nil, - testMockS3WithPutObjectError(new(throttleErr)), + testMockS3WithPutObjectError(TestAwsS3Error("ThrottlingException", "throttling exception")), ), ), }, - expectedErrContains: "error putting object into s3: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to put object", expectedErrCode: codes.Unavailable, }, { @@ -2874,12 +2959,12 @@ func TestStoragePlugin_DeleteObjects(t *testing.T) { withTestS3APIFunc( newTestMockS3( nil, - testMockS3WithDeleteObjectError(errors.New(testDeleteObjectErr)), + testMockS3WithDeleteObjectError(TestAwsS3Error("AccessDenied", "not authorized to perform HeadObject action")), ), ), }, - expectedErrContains: "error deleting S3 object: test error for DeleteObject", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to delete object", + expectedErrCode: codes.PermissionDenied, }, { name: "ListObjectV2 error", @@ -2889,12 +2974,12 @@ func TestStoragePlugin_DeleteObjects(t *testing.T) { withTestS3APIFunc( newTestMockS3( nil, - testMockS3WithListObjectsV2Error(errors.New(testListObjectV2Err)), + testMockS3WithListObjectsV2Error(TestAwsS3Error("AccessDenied", "not authorized to perform HeadObject action")), ), ), }, - expectedErrContains: "error iterating S3 bucket contents: test error for ListObjectV2", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to list objects", + expectedErrCode: codes.PermissionDenied, }, { name: "DeleteObjects error", @@ -2907,17 +2992,17 @@ func TestStoragePlugin_DeleteObjects(t *testing.T) { testMockS3WithListObjectsV2Output(&s3.ListObjectsV2Output{ IsTruncated: false, Contents: []s3types.Object{ - s3types.Object{ + { Key: aws.String("abc/abc"), }, }, }), - testMockS3WithDeleteObjectsError(errors.New(testDeleteObjectsErr)), + testMockS3WithDeleteObjectsError(TestAwsS3Error("AccessDenied", "not authorized to perform HeadObject action")), ), ), }, - expectedErrContains: "error deleting S3 object(s): test error for DeleteObjects", - expectedErrCode: codes.Internal, + expectedErrContains: "aws service s3: invalid credentials error: failed to delete objects", + expectedErrCode: codes.PermissionDenied, }, { name: "DeleteObject throttle error", @@ -2927,11 +3012,11 @@ func TestStoragePlugin_DeleteObjects(t *testing.T) { withTestS3APIFunc( newTestMockS3( nil, - testMockS3WithDeleteObjectError(new(throttleErr)), + testMockS3WithDeleteObjectError(TestAwsS3Error("ThrottlingException", "throttling exception")), ), ), }, - expectedErrContains: "error deleting S3 object: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to delete object", expectedErrCode: codes.Unavailable, }, { @@ -2942,11 +3027,11 @@ func TestStoragePlugin_DeleteObjects(t *testing.T) { withTestS3APIFunc( newTestMockS3( nil, - testMockS3WithListObjectsV2Error(new(throttleErr)), + testMockS3WithListObjectsV2Error(TestAwsS3Error("ThrottlingException", "throttling exception")), ), ), }, - expectedErrContains: "error iterating S3 bucket contents: ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to list objects", expectedErrCode: codes.Unavailable, }, { @@ -2965,11 +3050,11 @@ func TestStoragePlugin_DeleteObjects(t *testing.T) { }, }, }), - testMockS3WithDeleteObjectsError(new(throttleErr)), + testMockS3WithDeleteObjectsError(TestAwsS3Error("ThrottlingException", "throttling exception")), ), ), }, - expectedErrContains: "error deleting S3 object(s): ThrottlingException", + expectedErrContains: "aws service s3: throttling error: failed to delete objects", expectedErrCode: codes.Unavailable, }, { diff --git a/plugin/service/storage/testing.go b/plugin/service/storage/testing.go index d1ef852..68cde07 100644 --- a/plugin/service/storage/testing.go +++ b/plugin/service/storage/testing.go @@ -14,6 +14,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/sts" + "github.com/aws/smithy-go" "github.com/hashicorp/boundary-plugin-aws/internal/credential" "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/storagebuckets" pb "github.com/hashicorp/boundary/sdk/pbs/plugin" @@ -28,25 +29,8 @@ const ( testGetUserErr = "test error for GetUser" testGetCallerIdentityErr = "test error for GetCallerIdentity" testDeleteAccessKeyErr = "test error for DeleteAccessKey" - testGetObjectErr = "test error for GetObject" - testPutObjectErr = "test error for PutObject" - testHeadObjectErr = "test error for HeadObject" - testListObjectV2Err = "test error for ListObjectV2" - testDeleteObjectErr = "test error for DeleteObject" - testDeleteObjectsErr = "test error for DeleteObjects" ) -// throttleErr is a mocked error used for testing the aws s3 client -type throttleErr struct{} - -func (e *throttleErr) ErrorCode() string { - return "ThrottlingException" -} - -func (e *throttleErr) Error() string { - return "ThrottlingException" -} - type testMockS3State struct { GetObjectCalled bool GetObjectInputParams *s3.GetObjectInput @@ -414,3 +398,15 @@ func validSTSMock() []credential.AwsCredentialPersistedStateOption { }), } } + +// TestAwsS3Error returns an s3 api error +func TestAwsS3Error(code, msg string) error { + return &smithy.OperationError{ + ServiceID: "s3", + Err: &smithy.GenericAPIError{ + Code: code, + Message: msg, + Fault: smithy.FaultServer, + }, + } +}