Fix access denied error handling for GET requests #1340

Merged
fyrchik merged 1 commit from dstepanov-yadro/frostfs-node:fix/get_ape_err_handling into master 2024-08-28 12:15:05 +00:00
3 changed files with 29 additions and 12 deletions

View file

@ -2,9 +2,11 @@ package getsvc
import (
"context"
"errors"
"git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs"
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util"
apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status"
objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object"
"go.uber.org/zap"
)
@ -120,6 +122,12 @@ func (exec *request) analyzeStatus(ctx context.Context, execCnr bool) {
exec.log.Debug(logs.OperationFinishedWithError,
zap.Error(exec.err),
)
var errAccessDenied *apistatus.ObjectAccessDenied
if execCnr && errors.As(exec.err, &errAccessDenied) {
// Local get can't return access denied error, so this error was returned by
// write to the output stream. So there is no need to try to find object on other nodes.
return
}
if execCnr {
exec.executeOnContainer(ctx)

View file

@ -31,6 +31,7 @@ func (r *request) processNode(ctx context.Context, info client.NodeInfo) bool {
var errECInfo *objectSDK.ECInfoError
var errRemoved *apistatus.ObjectAlreadyRemoved
var errOutOfRange *apistatus.ObjectOutOfRange
var errAccessDenied *apistatus.ObjectAccessDenied
switch {
default:
@ -38,7 +39,11 @@ func (r *request) processNode(ctx context.Context, info client.NodeInfo) bool {
if r.status != statusEC {
// for raw requests, continue to collect other parts
r.status = statusUndefined
r.err = new(apistatus.ObjectNotFound)
if errors.As(err, &errAccessDenied) {
r.err = err
} else {
r.err = new(apistatus.ObjectNotFound)
}
}
return false
case err == nil:

View file

@ -23,12 +23,14 @@ import (
)
type getRequestForwarder struct {
OnceResign sync.Once
OnceHeaderSending sync.Once
GlobalProgress int
Key *ecdsa.PrivateKey
Request *objectV2.GetRequest
Stream *streamObjectWriter
OnceResign sync.Once
GlobalProgress int
Key *ecdsa.PrivateKey
Request *objectV2.GetRequest
Stream *streamObjectWriter
headerSent bool
headerSentGuard sync.Mutex
}
func (f *getRequestForwarder) forwardRequestToNode(ctx context.Context, addr network.Address, c client.MultiAddressClient, pubkey []byte) (*objectSDK.Object, error) {
@ -83,13 +85,15 @@ func (f *getRequestForwarder) writeHeader(ctx context.Context, v *objectV2.GetOb
obj.SetSignature(v.GetSignature())
obj.SetHeader(v.GetHeader())
var err error
f.OnceHeaderSending.Do(func() {
err = f.Stream.WriteHeader(ctx, objectSDK.NewFromV2(obj))
})
if err != nil {
f.headerSentGuard.Lock()
fyrchik marked this conversation as resolved
Review

Missed this in support branch, could you explain, how is this different from sync.Once?

Missed this in support branch, could you explain, how is this different from `sync.Once`?
Review

sync.Once doesn't take err into account: in case of error sync.Once will not run second time.

`sync.Once` doesn't take `err` into account: in case of error `sync.Once` will not run second time.
defer f.headerSentGuard.Unlock()
if f.headerSent {
return nil
}
if err := f.Stream.WriteHeader(ctx, objectSDK.NewFromV2(obj)); err != nil {
return errCouldNotWriteObjHeader(err)
}
f.headerSent = true
return nil
}