From 6e91074b50b0838021bb3dd12a227329104d9203 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Thu, 21 Apr 2022 11:49:56 +0300 Subject: [PATCH] [#367] Check errors using status Signed-off-by: Denis Kirillov --- api/handler/delete.go | 14 +++++++++-- api/layer/container.go | 4 +-- api/layer/object.go | 3 ++- internal/neofs/neofs.go | 48 +++++++++++++++++++----------------- internal/neofs/neofs_test.go | 25 +++++++++++++++++++ 5 files changed, 67 insertions(+), 27 deletions(-) create mode 100644 internal/neofs/neofs_test.go diff --git a/api/handler/delete.go b/api/handler/delete.go index 6c0198cdb..aa020c94b 100644 --- a/api/handler/delete.go +++ b/api/handler/delete.go @@ -4,12 +4,12 @@ import ( "encoding/xml" "net/http" "strconv" - "strings" "github.com/nspcc-dev/neofs-s3-gw/api" "github.com/nspcc-dev/neofs-s3-gw/api/data" "github.com/nspcc-dev/neofs-s3-gw/api/errors" "github.com/nspcc-dev/neofs-s3-gw/api/layer" + apistatus "github.com/nspcc-dev/neofs-sdk-go/client/status" oid "github.com/nspcc-dev/neofs-sdk-go/object/id" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -86,7 +86,7 @@ func (h *handler) DeleteObjectHandler(w http.ResponseWriter, r *http.Request) { err = deletedObject.Error } if err != nil { - if strings.Contains(err.Error(), "object is locked") { + if isErrObjectLocked(err) { h.logAndSendError(w, "object is locked", reqInfo, errors.GetAPIError(errors.ErrAccessDenied)) return } @@ -140,6 +140,16 @@ func (h *handler) DeleteObjectHandler(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } +func isErrObjectLocked(err error) bool { + switch err.(type) { + default: + return false + case apistatus.ObjectLocked, + *apistatus.ObjectLocked: + return true + } +} + // DeleteMultipleObjectsHandler handles multiple delete requests. func (h *handler) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *http.Request) { reqInfo := api.GetReqInfo(r.Context()) diff --git a/api/layer/container.go b/api/layer/container.go index dce1a40e5..a8106d470 100644 --- a/api/layer/container.go +++ b/api/layer/container.go @@ -3,7 +3,6 @@ package layer import ( "context" "strconv" - "strings" "time" "github.com/nspcc-dev/neofs-s3-gw/api" @@ -11,6 +10,7 @@ import ( "github.com/nspcc-dev/neofs-s3-gw/api/errors" "github.com/nspcc-dev/neofs-s3-gw/api/layer/neofs" "github.com/nspcc-dev/neofs-sdk-go/acl" + "github.com/nspcc-dev/neofs-sdk-go/client" "github.com/nspcc-dev/neofs-sdk-go/container" cid "github.com/nspcc-dev/neofs-sdk-go/container/id" "github.com/nspcc-dev/neofs-sdk-go/eacl" @@ -47,7 +47,7 @@ func (n *layer) containerInfo(ctx context.Context, idCnr *cid.ID) (*data.BucketI if err != nil { log.Error("could not fetch container", zap.Error(err)) - if strings.Contains(err.Error(), "container not found") { + if client.IsErrContainerNotFound(err) { return nil, errors.GetAPIError(errors.ErrNoSuchBucket) } return nil, err diff --git a/api/layer/object.go b/api/layer/object.go index 551380ee2..d404cd128 100644 --- a/api/layer/object.go +++ b/api/layer/object.go @@ -14,6 +14,7 @@ import ( "github.com/nspcc-dev/neofs-s3-gw/api/data" apiErrors "github.com/nspcc-dev/neofs-s3-gw/api/errors" "github.com/nspcc-dev/neofs-s3-gw/api/layer/neofs" + "github.com/nspcc-dev/neofs-sdk-go/client" cid "github.com/nspcc-dev/neofs-sdk-go/container/id" "github.com/nspcc-dev/neofs-sdk-go/object" "github.com/nspcc-dev/neofs-sdk-go/object/address" @@ -416,7 +417,7 @@ func (n *layer) headVersion(ctx context.Context, bkt *data.BucketInfo, p *HeadOb meta, err := n.objectHead(ctx, bkt.CID, id) if err != nil { - if strings.Contains(err.Error(), "not found") { + if client.IsErrObjectNotFound(err) { return nil, apiErrors.GetAPIError(apiErrors.ErrNoSuchVersion) } return nil, err diff --git a/internal/neofs/neofs.go b/internal/neofs/neofs.go index f3b483b92..1d759adbe 100644 --- a/internal/neofs/neofs.go +++ b/internal/neofs/neofs.go @@ -9,13 +9,13 @@ import ( "io" "math" "strconv" - "strings" "time" objectv2 "github.com/nspcc-dev/neofs-api-go/v2/object" "github.com/nspcc-dev/neofs-s3-gw/api/layer/neofs" "github.com/nspcc-dev/neofs-s3-gw/authmate" "github.com/nspcc-dev/neofs-s3-gw/creds/tokens" + apistatus "github.com/nspcc-dev/neofs-sdk-go/client/status" "github.com/nspcc-dev/neofs-sdk-go/container" cid "github.com/nspcc-dev/neofs-sdk-go/container/id" "github.com/nspcc-dev/neofs-sdk-go/eacl" @@ -292,9 +292,8 @@ func (x *NeoFS) SelectObjects(ctx context.Context, prm neofs.PrmObjectSelect) ([ return false }) if err != nil { - // TODO: (neofs-s3-gw#367) use NeoFS SDK API to check the status return - if strings.Contains(err.Error(), "access to operation") && strings.Contains(err.Error(), "is denied by") { - return nil, neofs.ErrAccessDenied + if reason, ok := isErrAccessDenied(err); ok { + return nil, fmt.Errorf("%w: %s", neofs.ErrAccessDenied, reason) } return nil, fmt.Errorf("read object list: %w", err) @@ -312,9 +311,8 @@ type payloadReader struct { func (x payloadReader) Read(p []byte) (int, error) { n, err := x.ReadCloser.Read(p) if err != nil { - // TODO: (neofs-s3-gw#367) use NeoFS SDK API to check the status return - if strings.Contains(err.Error(), "access to operation") && strings.Contains(err.Error(), "is denied by") { - return n, neofs.ErrAccessDenied + if reason, ok := isErrAccessDenied(err); ok { + return n, fmt.Errorf("%w: %s", neofs.ErrAccessDenied, reason) } } @@ -340,9 +338,8 @@ func (x *NeoFS) ReadObject(ctx context.Context, prm neofs.PrmObjectRead) (*neofs if prm.WithPayload { res, err := x.pool.GetObject(ctx, prmGet) if err != nil { - // TODO: (neofs-s3-gw#367) use NeoFS SDK API to check the status return - if strings.Contains(err.Error(), "access to operation") && strings.Contains(err.Error(), "is denied by") { - return nil, neofs.ErrAccessDenied + if reason, ok := isErrAccessDenied(err); ok { + return nil, fmt.Errorf("%w: %s", neofs.ErrAccessDenied, reason) } return nil, fmt.Errorf("init full object reading via connection pool: %w", err) @@ -373,9 +370,8 @@ func (x *NeoFS) ReadObject(ctx context.Context, prm neofs.PrmObjectRead) (*neofs hdr, err := x.pool.HeadObject(ctx, prmHead) if err != nil { - // TODO: (neofs-s3-gw#367) use NeoFS SDK API to check the status return - if strings.Contains(err.Error(), "access to operation") && strings.Contains(err.Error(), "is denied by") { - return nil, neofs.ErrAccessDenied + if reason, ok := isErrAccessDenied(err); ok { + return nil, fmt.Errorf("%w: %s", neofs.ErrAccessDenied, reason) } return nil, fmt.Errorf("read object header via connection pool: %w", err) @@ -387,9 +383,8 @@ func (x *NeoFS) ReadObject(ctx context.Context, prm neofs.PrmObjectRead) (*neofs } else if prm.PayloadRange[0]+prm.PayloadRange[1] == 0 { res, err := x.pool.GetObject(ctx, prmGet) if err != nil { - // TODO: (neofs-s3-gw#367) use NeoFS SDK API to check the status return - if strings.Contains(err.Error(), "access to operation") && strings.Contains(err.Error(), "is denied by") { - return nil, neofs.ErrAccessDenied + if reason, ok := isErrAccessDenied(err); ok { + return nil, fmt.Errorf("%w: %s", neofs.ErrAccessDenied, reason) } return nil, fmt.Errorf("init full payload range reading via connection pool: %w", err) @@ -413,9 +408,8 @@ func (x *NeoFS) ReadObject(ctx context.Context, prm neofs.PrmObjectRead) (*neofs res, err := x.pool.ObjectRange(ctx, prmRange) if err != nil { - // TODO: (neofs-s3-gw#367) use NeoFS SDK API to check the status return - if strings.Contains(err.Error(), "access to operation") && strings.Contains(err.Error(), "is denied by") { - return nil, neofs.ErrAccessDenied + if reason, ok := isErrAccessDenied(err); ok { + return nil, fmt.Errorf("%w: %s", neofs.ErrAccessDenied, reason) } return nil, fmt.Errorf("init payload range reading via connection pool: %w", err) @@ -443,9 +437,8 @@ func (x *NeoFS) DeleteObject(ctx context.Context, prm neofs.PrmObjectDelete) err err := x.pool.DeleteObject(ctx, prmDelete) if err != nil { - // TODO: (neofs-s3-gw#367) use NeoFS SDK API to check the status return - if strings.Contains(err.Error(), "access to operation") && strings.Contains(err.Error(), "is denied by") { - return neofs.ErrAccessDenied + if reason, ok := isErrAccessDenied(err); ok { + return fmt.Errorf("%w: %s", neofs.ErrAccessDenied, reason) } return fmt.Errorf("mark object removal via connection pool: %w", err) @@ -454,6 +447,17 @@ func (x *NeoFS) DeleteObject(ctx context.Context, prm neofs.PrmObjectDelete) err return nil } +func isErrAccessDenied(err error) (string, bool) { + switch err := err.(type) { + default: + return "", false + case apistatus.ObjectAccessDenied: + return err.Reason(), true + case *apistatus.ObjectAccessDenied: + return err.Reason(), true + } +} + // ResolverNeoFS represents virtual connection to the NeoFS network. // It implements resolver.NeoFS. type ResolverNeoFS struct { diff --git a/internal/neofs/neofs_test.go b/internal/neofs/neofs_test.go new file mode 100644 index 000000000..04f56ccc8 --- /dev/null +++ b/internal/neofs/neofs_test.go @@ -0,0 +1,25 @@ +package neofs + +import ( + "fmt" + "testing" + + "github.com/nspcc-dev/neofs-s3-gw/api/layer/neofs" + apistatus "github.com/nspcc-dev/neofs-sdk-go/client/status" + "github.com/stretchr/testify/require" +) + +func TestErrorChecking(t *testing.T) { + reason := "some reason" + err := new(apistatus.ObjectAccessDenied) + err.WriteReason(reason) + + var wrappedError error + + if fetchedReason, ok := isErrAccessDenied(err); ok { + wrappedError = fmt.Errorf("%w: %s", neofs.ErrAccessDenied, fetchedReason) + } + + require.ErrorIs(t, wrappedError, neofs.ErrAccessDenied) + require.Contains(t, wrappedError.Error(), reason) +}