[#488] middleware/policy: Add frostfs-to-s3 error transformation

Signed-off-by: Nikita Zinkevich <n.zinkevich@yadro.com>
This commit is contained in:
Nikita Zinkevich 2024-09-27 12:26:08 +03:00
parent 9fadfbbc2f
commit bc17ab5e47
10 changed files with 143 additions and 111 deletions

View file

@ -1,10 +1,13 @@
package errors
import (
"errors"
"fmt"
"net/http"
frosterrors "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/frostfs/errors"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/frostfs"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/tree"
frosterr "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/frostfs/errors"
)
type (
@ -1765,7 +1768,7 @@ var errorCodes = errorCodeMap{
// IsS3Error checks if the provided error is a specific s3 error.
func IsS3Error(err error, code ErrorCode) bool {
err = frosterrors.UnwrapErr(err)
err = frosterr.UnwrapErr(err)
e, ok := err.(Error)
return ok && e.ErrCode == code
}
@ -1802,6 +1805,26 @@ func GetAPIErrorWithError(code ErrorCode, err error) Error {
return errorCodes.toAPIErrWithErr(code, err)
}
// TransformToS3Error converts FrostFS error to the corresponding S3 error type.
func TransformToS3Error(err error) error {
err = frosterr.UnwrapErr(err) // this wouldn't work with errors.Join
var s3err Error
if errors.As(err, &s3err) {
return err
}
if errors.Is(err, frostfs.ErrAccessDenied) ||
errors.Is(err, tree.ErrNodeAccessDenied) {
return GetAPIError(ErrAccessDenied)
}
if errors.Is(err, frostfs.ErrGatewayTimeout) {
return GetAPIError(ErrGatewayTimeout)
}
return GetAPIError(ErrInternalError)
}
// ObjectError -- error that is linked to a specific object.
type ObjectError struct {
Err error

View file

@ -2,7 +2,12 @@ package errors
import (
"errors"
"fmt"
"testing"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/frostfs"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/tree"
"github.com/stretchr/testify/require"
)
func BenchmarkErrCode(b *testing.B) {
@ -24,3 +29,56 @@ func BenchmarkErrorsIs(b *testing.B) {
}
}
}
func TestTransformS3Errors(t *testing.T) {
for _, tc := range []struct {
name string
err error
expected ErrorCode
}{
{
name: "simple std error to internal error",
err: errors.New("some error"),
expected: ErrInternalError,
},
{
name: "layer access denied error to s3 access denied error",
err: frostfs.ErrAccessDenied,
expected: ErrAccessDenied,
},
{
name: "wrapped layer access denied error to s3 access denied error",
err: fmt.Errorf("wrap: %w", frostfs.ErrAccessDenied),
expected: ErrAccessDenied,
},
{
name: "layer node access denied error to s3 access denied error",
err: tree.ErrNodeAccessDenied,
expected: ErrAccessDenied,
},
{
name: "layer gateway timeout error to s3 gateway timeout error",
err: frostfs.ErrGatewayTimeout,
expected: ErrGatewayTimeout,
},
{
name: "s3 error to s3 error",
err: GetAPIError(ErrInvalidPart),
expected: ErrInvalidPart,
},
{
name: "wrapped s3 error to s3 error",
err: fmt.Errorf("wrap: %w", GetAPIError(ErrInvalidPart)),
expected: ErrInvalidPart,
},
} {
t.Run(tc.name, func(t *testing.T) {
err := TransformToS3Error(tc.err)
s3err, ok := err.(Error)
require.True(t, ok, "error must be s3 error")
require.Equalf(t, tc.expected, s3err.ErrCode,
"expected: '%s', got: '%s'",
GetAPIError(tc.expected).Code, GetAPIError(s3err.ErrCode).Code)
})
}
}

View file

@ -12,10 +12,7 @@ import (
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data"
apierr "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/frostfs"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/tree"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware"
frosterrors "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/frostfs/errors"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/logs"
cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
"go.opentelemetry.io/otel/trace"
@ -32,7 +29,7 @@ func (h *handler) reqLogger(ctx context.Context) *zap.Logger {
func (h *handler) logAndSendError(w http.ResponseWriter, logText string, reqInfo *middleware.ReqInfo, err error, additional ...zap.Field) {
err = handleDeleteMarker(w, err)
if code, wrErr := middleware.WriteErrorResponse(w, reqInfo, transformToS3Error(err)); wrErr != nil {
if code, wrErr := middleware.WriteErrorResponse(w, reqInfo, apierr.TransformToS3Error(err)); wrErr != nil {
additional = append(additional, zap.NamedError("write_response_error", wrErr))
} else {
additional = append(additional, zap.Int("status", code))
@ -62,24 +59,6 @@ func handleDeleteMarker(w http.ResponseWriter, err error) error {
return fmt.Errorf("%w: %s", apierr.GetAPIError(target.ErrorCode), err)
}
func transformToS3Error(err error) error {
err = frosterrors.UnwrapErr(err) // this wouldn't work with errors.Join
if _, ok := err.(apierr.Error); ok {
return err
}
if errors.Is(err, frostfs.ErrAccessDenied) ||
errors.Is(err, tree.ErrNodeAccessDenied) {
return apierr.GetAPIError(apierr.ErrAccessDenied)
}
if errors.Is(err, frostfs.ErrGatewayTimeout) {
return apierr.GetAPIError(apierr.ErrGatewayTimeout)
}
return apierr.GetAPIError(apierr.ErrInternalError)
}
func (h *handler) ResolveBucket(ctx context.Context, bucket string) (*data.BucketInfo, error) {
return h.obj.GetBucketInfo(ctx, bucket)
}

View file

@ -1,65 +1 @@
package handler
import (
"errors"
"fmt"
"testing"
s3errors "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/frostfs"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/tree"
"github.com/stretchr/testify/require"
)
func TestTransformS3Errors(t *testing.T) {
for _, tc := range []struct {
name string
err error
expected s3errors.ErrorCode
}{
{
name: "simple std error to internal error",
err: errors.New("some error"),
expected: s3errors.ErrInternalError,
},
{
name: "layer access denied error to s3 access denied error",
err: frostfs.ErrAccessDenied,
expected: s3errors.ErrAccessDenied,
},
{
name: "wrapped layer access denied error to s3 access denied error",
err: fmt.Errorf("wrap: %w", frostfs.ErrAccessDenied),
expected: s3errors.ErrAccessDenied,
},
{
name: "layer node access denied error to s3 access denied error",
err: tree.ErrNodeAccessDenied,
expected: s3errors.ErrAccessDenied,
},
{
name: "layer gateway timeout error to s3 gateway timeout error",
err: frostfs.ErrGatewayTimeout,
expected: s3errors.ErrGatewayTimeout,
},
{
name: "s3 error to s3 error",
err: s3errors.GetAPIError(s3errors.ErrInvalidPart),
expected: s3errors.ErrInvalidPart,
},
{
name: "wrapped s3 error to s3 error",
err: fmt.Errorf("wrap: %w", s3errors.GetAPIError(s3errors.ErrInvalidPart)),
expected: s3errors.ErrInvalidPart,
},
} {
t.Run(tc.name, func(t *testing.T) {
err := transformToS3Error(tc.err)
s3err, ok := err.(s3errors.Error)
require.True(t, ok, "error must be s3 error")
require.Equalf(t, tc.expected, s3err.ErrCode,
"expected: '%s', got: '%s'",
s3errors.GetAPIError(tc.expected).Code, s3errors.GetAPIError(s3err.ErrCode).Code)
})
}
}

View file

@ -53,7 +53,7 @@ type (
anonKey AnonymousKey
resolver BucketResolver
cache *Cache
treeService tree.TreeService
treeService tree.Service
features FeatureSettings
gateKey *keys.PrivateKey
corsCnrInfo *data.BucketInfo
@ -66,7 +66,7 @@ type (
Cache *Cache
AnonKey AnonymousKey
Resolver BucketResolver
TreeService tree.TreeService
TreeService tree.Service
Features FeatureSettings
GateKey *keys.PrivateKey
CORSCnrInfo *data.BucketInfo

View file

@ -8,8 +8,8 @@ import (
oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
)
// TreeService provide interface to interact with tree service using s3 data models.
type TreeService interface {
// Service provide interface to interact with tree service using s3 data models.
type Service interface {
// PutSettingsNode update or create new settings node in tree service.
PutSettingsNode(ctx context.Context, bktInfo *data.BucketInfo, settings *data.BucketSettings) error

View file

@ -58,7 +58,6 @@ func Auth(center Center, log *zap.Logger) Func {
} else {
reqLogOrDefault(ctx, log).Error(logs.FailedToPassAuthentication, zap.Error(err))
err = frosterr.UnwrapErr(err)
log.Info("error", zap.Error(err), zap.String("code", err.(apierr.Error).Code), zap.String("description", err.(apierr.Error).Description))
if _, ok := err.(apierr.Error); !ok {
err = apierr.GetAPIError(apierr.ErrAccessDenied)
}

View file

@ -12,7 +12,6 @@ import (
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data"
apierr "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors"
frostfsErrors "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/frostfs/errors"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/logs"
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine"
@ -85,7 +84,7 @@ func PolicyCheck(cfg PolicyConfig) Func {
ctx := r.Context()
if err := policyCheck(r, cfg); err != nil {
reqLogOrDefault(ctx, cfg.Log).Error(logs.PolicyValidationFailed, zap.Error(err))
err = frostfsErrors.UnwrapErr(err)
err = apierr.TransformToS3Error(err)
if _, wrErr := WriteErrorResponse(w, GetReqInfo(ctx), err); wrErr != nil {
reqLogOrDefault(ctx, cfg.Log).Error(logs.FailedToWriteResponse, zap.Error(wrErr))
}

View file

@ -40,8 +40,9 @@ type centerMock struct {
t *testing.T
anon bool
noAuthHeader bool
isError bool
err error
attrs []object.Attribute
key *keys.PrivateKey
}
func (c *centerMock) Authenticate(*http.Request) (*middleware.Box, error) {
@ -49,8 +50,8 @@ func (c *centerMock) Authenticate(*http.Request) (*middleware.Box, error) {
return nil, middleware.ErrNoAuthorizationHeader
}
if c.isError {
return nil, fmt.Errorf("some error")
if c.err != nil {
return nil, c.err
}
var token *bearer.Token
@ -58,8 +59,12 @@ func (c *centerMock) Authenticate(*http.Request) (*middleware.Box, error) {
if !c.anon {
bt := bearertest.Token()
token = &bt
key, err := keys.NewPrivateKey()
require.NoError(c.t, err)
key := c.key
if key == nil {
var err error
key, err = keys.NewPrivateKey()
require.NoError(c.t, err)
}
require.NoError(c.t, token.Sign(key.PrivateKey))
}
@ -151,22 +156,21 @@ func (m *xmlMock) NewXMLDecoder(r io.Reader) *xml.Decoder {
}
type resourceTaggingMock struct {
bucketTags map[string]string
objectTags map[string]string
noSuchObjectKey bool
noSuchBucketKey bool
bucketTags map[string]string
objectTags map[string]string
err error
}
func (m *resourceTaggingMock) GetBucketTagging(context.Context, *data.BucketInfo) (map[string]string, error) {
if m.noSuchBucketKey {
return nil, apierr.GetAPIError(apierr.ErrNoSuchKey)
if m.err != nil {
return nil, m.err
}
return m.bucketTags, nil
}
func (m *resourceTaggingMock) GetObjectTagging(context.Context, *data.GetObjectTaggingParams) (string, map[string]string, error) {
if m.noSuchObjectKey {
return "", nil, apierr.GetAPIError(apierr.ErrNoSuchKey)
if m.err != nil {
return "", nil, m.err
}
return "", m.objectTags, nil
}

View file

@ -15,6 +15,7 @@ import (
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data"
apierr "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/frostfs"
s3middleware "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware"
"git.frostfs.info/TrueCloudLab/frostfs-s3-gw/metrics"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object"
@ -26,6 +27,7 @@ import (
"git.frostfs.info/TrueCloudLab/policy-engine/schema/s3"
"github.com/go-chi/chi/v5"
"github.com/go-chi/chi/v5/middleware"
"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/require"
"go.uber.org/zap/zaptest"
@ -274,6 +276,36 @@ func TestPolicyCheckerReqTypeDetermination(t *testing.T) {
})
}
func TestPolicyCheckFrostfsErrors(t *testing.T) {
chiRouter := prepareRouter(t)
ns1, bktName1, objName1 := "", "bucket", "object"
createBucket(chiRouter, ns1, bktName1)
key, err := keys.NewPrivateKey()
require.NoError(t, err)
chiRouter.cfg.Center.(*centerMock).key = key
chiRouter.cfg.MiddlewareSettings.(*middlewareSettingsMock).denyByDefault = true
ruleChain := &chain.Chain{
ID: chain.ID("id"),
Rules: []chain.Rule{{
Status: chain.Allow,
Actions: chain.Actions{Names: []string{"*"}},
Resources: chain.Resources{Names: []string{fmt.Sprintf(s3.ResourceFormatS3BucketObjects, bktName1)}},
}},
}
_, _, err = chiRouter.policyChecker.MorphRuleChainStorage().AddMorphRuleChain(chain.S3, engine.UserTarget(ns1+":"+key.Address()), ruleChain)
require.NoError(t, err)
// check we can access 'bucket' in default namespace
putObject(chiRouter, ns1, bktName1, objName1, nil)
chiRouter.cfg.Center.(*centerMock).anon = true
chiRouter.cfg.Tagging.(*resourceTaggingMock).err = frostfs.ErrAccessDenied
getObjectErr(chiRouter, ns1, bktName1, objName1, apierr.ErrAccessDenied)
}
func TestDefaultBehaviorPolicyChecker(t *testing.T) {
chiRouter := prepareRouter(t)
ns, bktName := "", "bucket"
@ -524,11 +556,10 @@ func TestResourceTagsCheck(t *testing.T) {
listObjectsV1Err(router, ns, bktName, "", "", "", apierr.ErrNoSuchBucket)
router.cfg.Tagging.(*resourceTaggingMock).noSuchBucketKey = true
router.cfg.Tagging.(*resourceTaggingMock).err = apierr.GetAPIError(apierr.ErrNoSuchKey)
createBucket(router, ns, bktName)
getBucketErr(router, ns, bktName, apierr.ErrNoSuchKey)
router.cfg.Tagging.(*resourceTaggingMock).noSuchObjectKey = true
createBucket(router, ns, bktName)
getObjectErr(router, ns, bktName, objName, apierr.ErrNoSuchKey)
})
@ -826,8 +857,11 @@ func TestAuthenticate(t *testing.T) {
createBucket(chiRouter, "", "bkt-2")
chiRouter = prepareRouter(t)
chiRouter.cfg.Center.(*centerMock).isError = true
chiRouter.cfg.Center.(*centerMock).err = apierr.GetAPIError(apierr.ErrAccessDenied)
createBucketErr(chiRouter, "", "bkt-3", nil, apierr.ErrAccessDenied)
chiRouter.cfg.Center.(*centerMock).err = frostfs.ErrGatewayTimeout
createBucketErr(chiRouter, "", "bkt-3", nil, apierr.ErrGatewayTimeout)
}
func TestFrostFSIDValidation(t *testing.T) {