forked from TrueCloudLab/frostfs-node
[#919] ape: Improve error messages in ape service
* Wrap all APE middleware errors in apeErr that makes errors more explicit with status AccessDenied. * Use denyingRuleErr for denying status from chain router. Signed-off-by: Airat Arifullin <a.arifullin@yadro.com>
This commit is contained in:
parent
f526f49995
commit
f2f3294fc3
4 changed files with 32 additions and 28 deletions
|
@ -5,7 +5,6 @@ import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
|
||||||
objectV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object"
|
objectV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object"
|
||||||
apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status"
|
|
||||||
cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
|
cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
|
||||||
oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
|
oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
|
||||||
apechain "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
|
apechain "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
|
||||||
|
@ -70,13 +69,5 @@ func (c *checkerImpl) CheckAPE(ctx context.Context, prm Prm) error {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
return apeErr(prm.Method, status)
|
return fmt.Errorf("found denying rule for %s: %s", prm.Method, status)
|
||||||
}
|
|
||||||
|
|
||||||
const accessDeniedAPEReasonFmt = "access to operation %s is denied by access policy engine: %s"
|
|
||||||
|
|
||||||
func apeErr(op string, status apechain.Status) error {
|
|
||||||
errAccessDenied := &apistatus.ObjectAccessDenied{}
|
|
||||||
errAccessDenied.WriteReason(fmt.Sprintf(accessDeniedAPEReasonFmt, op, status.String()))
|
|
||||||
return errAccessDenied
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -308,7 +308,8 @@ func TestAPECheck(t *testing.T) {
|
||||||
ms := inmemory.NewInmemoryMorphRuleChainStorage()
|
ms := inmemory.NewInmemoryMorphRuleChainStorage()
|
||||||
|
|
||||||
ls.AddOverride(chain.Ingress, policyengine.ContainerTarget(test.container), &chain.Chain{
|
ls.AddOverride(chain.Ingress, policyengine.ContainerTarget(test.container), &chain.Chain{
|
||||||
Rules: test.containerRules,
|
Rules: test.containerRules,
|
||||||
|
MatchType: chain.MatchTypeFirstMatch,
|
||||||
})
|
})
|
||||||
|
|
||||||
router := policyengine.NewDefaultChainRouterWithLocalOverrides(ms, ls)
|
router := policyengine.NewDefaultChainRouterWithLocalOverrides(ms, ls)
|
||||||
|
@ -336,8 +337,7 @@ func TestAPECheck(t *testing.T) {
|
||||||
|
|
||||||
err := checker.CheckAPE(context.Background(), prm)
|
err := checker.CheckAPE(context.Background(), prm)
|
||||||
if test.expectAPEErr {
|
if test.expectAPEErr {
|
||||||
aErr := apeErr(method, chain.AccessDenied)
|
require.Error(t, err)
|
||||||
require.ErrorAs(t, err, &aErr)
|
|
||||||
} else {
|
} else {
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
}
|
}
|
||||||
|
|
13
pkg/services/object/ape/errors.go
Normal file
13
pkg/services/object/ape/errors.go
Normal file
|
@ -0,0 +1,13 @@
|
||||||
|
package ape
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
|
||||||
|
apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status"
|
||||||
|
)
|
||||||
|
|
||||||
|
func toStatusErr(err error) error {
|
||||||
|
errAccessDenied := &apistatus.ObjectAccessDenied{}
|
||||||
|
errAccessDenied.WriteReason(fmt.Sprintf("ape denied request: %s", err.Error()))
|
||||||
|
return errAccessDenied
|
||||||
|
}
|
|
@ -69,7 +69,7 @@ func (g *getStreamBasicChecker) Send(resp *objectV2.GetResponse) error {
|
||||||
if partInit, ok := resp.GetBody().GetObjectPart().(*objectV2.GetObjectPartInit); ok {
|
if partInit, ok := resp.GetBody().GetObjectPart().(*objectV2.GetObjectPartInit); ok {
|
||||||
cnrID, objID, err := getAddressParamsSDK(partInit.GetHeader().GetContainerID(), partInit.GetObjectID())
|
cnrID, objID, err := getAddressParamsSDK(partInit.GetHeader().GetContainerID(), partInit.GetObjectID())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
prm := Prm{
|
prm := Prm{
|
||||||
|
@ -82,7 +82,7 @@ func (g *getStreamBasicChecker) Send(resp *objectV2.GetResponse) error {
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := g.apeChecker.CheckAPE(g.Context(), prm); err != nil {
|
if err := g.apeChecker.CheckAPE(g.Context(), prm); err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return g.GetObjectStream.Send(resp)
|
return g.GetObjectStream.Send(resp)
|
||||||
|
@ -103,12 +103,12 @@ func requestContext(ctx context.Context) (*objectSvc.RequestContext, error) {
|
||||||
func (c *Service) Get(request *objectV2.GetRequest, stream objectSvc.GetObjectStream) error {
|
func (c *Service) Get(request *objectV2.GetRequest, stream objectSvc.GetObjectStream) error {
|
||||||
cnrID, objID, err := getAddressParamsSDK(request.GetBody().GetAddress().GetContainerID(), request.GetBody().GetAddress().GetObjectID())
|
cnrID, objID, err := getAddressParamsSDK(request.GetBody().GetAddress().GetContainerID(), request.GetBody().GetAddress().GetObjectID())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
reqCtx, err := requestContext(stream.Context())
|
reqCtx, err := requestContext(stream.Context())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
err = c.apeChecker.CheckAPE(stream.Context(), Prm{
|
err = c.apeChecker.CheckAPE(stream.Context(), Prm{
|
||||||
|
@ -120,7 +120,7 @@ func (c *Service) Get(request *objectV2.GetRequest, stream objectSvc.GetObjectSt
|
||||||
SenderKey: hex.EncodeToString(reqCtx.SenderKey),
|
SenderKey: hex.EncodeToString(reqCtx.SenderKey),
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
return c.next.Get(request, &getStreamBasicChecker{
|
return c.next.Get(request, &getStreamBasicChecker{
|
||||||
|
@ -139,12 +139,12 @@ func (p *putStreamBasicChecker) Send(ctx context.Context, request *objectV2.PutR
|
||||||
if partInit, ok := request.GetBody().GetObjectPart().(*objectV2.PutObjectPartInit); ok {
|
if partInit, ok := request.GetBody().GetObjectPart().(*objectV2.PutObjectPartInit); ok {
|
||||||
reqCtx, err := requestContext(ctx)
|
reqCtx, err := requestContext(ctx)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
cnrID, objID, err := getAddressParamsSDK(partInit.GetHeader().GetContainerID(), partInit.GetObjectID())
|
cnrID, objID, err := getAddressParamsSDK(partInit.GetHeader().GetContainerID(), partInit.GetObjectID())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
prm := Prm{
|
prm := Prm{
|
||||||
|
@ -158,7 +158,7 @@ func (p *putStreamBasicChecker) Send(ctx context.Context, request *objectV2.PutR
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := p.apeChecker.CheckAPE(ctx, prm); err != nil {
|
if err := p.apeChecker.CheckAPE(ctx, prm); err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -244,13 +244,13 @@ func (c *Service) Search(request *objectV2.SearchRequest, stream objectSvc.Searc
|
||||||
var cnrID cid.ID
|
var cnrID cid.ID
|
||||||
if cnrV2 := request.GetBody().GetContainerID(); cnrV2 != nil {
|
if cnrV2 := request.GetBody().GetContainerID(); cnrV2 != nil {
|
||||||
if err := cnrID.ReadFromV2(*cnrV2); err != nil {
|
if err := cnrID.ReadFromV2(*cnrV2); err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
reqCtx, err := requestContext(stream.Context())
|
reqCtx, err := requestContext(stream.Context())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
err = c.apeChecker.CheckAPE(stream.Context(), Prm{
|
err = c.apeChecker.CheckAPE(stream.Context(), Prm{
|
||||||
|
@ -261,7 +261,7 @@ func (c *Service) Search(request *objectV2.SearchRequest, stream objectSvc.Searc
|
||||||
SenderKey: hex.EncodeToString(reqCtx.SenderKey),
|
SenderKey: hex.EncodeToString(reqCtx.SenderKey),
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
return c.next.Search(request, stream)
|
return c.next.Search(request, stream)
|
||||||
|
@ -301,12 +301,12 @@ func (c *Service) Delete(ctx context.Context, request *objectV2.DeleteRequest) (
|
||||||
func (c *Service) GetRange(request *objectV2.GetRangeRequest, stream objectSvc.GetObjectRangeStream) error {
|
func (c *Service) GetRange(request *objectV2.GetRangeRequest, stream objectSvc.GetObjectRangeStream) error {
|
||||||
cnrID, objID, err := getAddressParamsSDK(request.GetBody().GetAddress().GetContainerID(), request.GetBody().GetAddress().GetObjectID())
|
cnrID, objID, err := getAddressParamsSDK(request.GetBody().GetAddress().GetContainerID(), request.GetBody().GetAddress().GetObjectID())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
reqCtx, err := requestContext(stream.Context())
|
reqCtx, err := requestContext(stream.Context())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
err = c.apeChecker.CheckAPE(stream.Context(), Prm{
|
err = c.apeChecker.CheckAPE(stream.Context(), Prm{
|
||||||
|
@ -318,7 +318,7 @@ func (c *Service) GetRange(request *objectV2.GetRangeRequest, stream objectSvc.G
|
||||||
SenderKey: hex.EncodeToString(reqCtx.SenderKey),
|
SenderKey: hex.EncodeToString(reqCtx.SenderKey),
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return toStatusErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
return c.next.GetRange(request, stream)
|
return c.next.GetRange(request, stream)
|
||||||
|
@ -381,7 +381,7 @@ func (c *Service) PutSingle(ctx context.Context, request *objectV2.PutSingleRequ
|
||||||
}
|
}
|
||||||
|
|
||||||
if err = c.apeChecker.CheckAPE(ctx, prm); err != nil {
|
if err = c.apeChecker.CheckAPE(ctx, prm); err != nil {
|
||||||
return nil, err
|
return nil, toStatusErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
return c.next.PutSingle(ctx, request)
|
return c.next.PutSingle(ctx, request)
|
||||||
|
|
Loading…
Reference in a new issue