From bfdff20cc6a972682365f62a4e4f08406dbb7526 Mon Sep 17 00:00:00 2001 From: Dmitrii Stepanov Date: Fri, 31 Mar 2023 14:49:40 +0300 Subject: [PATCH 1/7] [#193] getsvc: Resolve context linters Resolve containedctx and contextcheck linters. Signed-off-by: Dmitrii Stepanov --- pkg/services/object/get/assemble.go | 10 ++++------ pkg/services/object/get/container.go | 8 ++++---- pkg/services/object/get/exec.go | 21 +++++++-------------- pkg/services/object/get/get.go | 18 ++++++++---------- pkg/services/object/get/get_test.go | 2 +- pkg/services/object/get/local.go | 5 +++-- pkg/services/object/get/remote.go | 5 ++--- pkg/services/object/get/service.go | 4 +++- pkg/services/object/get/util.go | 14 +++++++------- 9 files changed, 39 insertions(+), 48 deletions(-) diff --git a/pkg/services/object/get/assemble.go b/pkg/services/object/get/assemble.go index ebae18eb5..db71df6a4 100644 --- a/pkg/services/object/get/assemble.go +++ b/pkg/services/object/get/assemble.go @@ -10,7 +10,7 @@ import ( "go.uber.org/zap" ) -func (exec *execCtx) assemble() { +func (exec *execCtx) assemble(ctx context.Context) { if !exec.canAssemble() { exec.log.Debug("can not assemble the object") return @@ -49,7 +49,7 @@ func (exec *execCtx) assemble() { zap.Uint64("range_length", exec.ctxRange().GetLength()), ) - obj, err := assembler.Assemble(exec.context(), exec.prm.objWriter) + obj, err := assembler.Assemble(ctx, exec.prm.objWriter) if err != nil { exec.log.Warn("failed to assemble splitted object", zap.Error(err), @@ -107,8 +107,7 @@ func (exec *execCtx) HeadObject(ctx context.Context, id oid.ID) (*objectSDK.Obje w := NewSimpleObjectWriter() prm.SetHeaderWriter(w) - //nolint: contextcheck - err := exec.svc.Head(exec.context(), prm) + err := exec.svc.Head(ctx, prm) if err != nil { return nil, err @@ -128,8 +127,7 @@ func (exec *execCtx) GetObject(ctx context.Context, id oid.ID, rng *objectSDK.Ra p.addr.SetContainer(exec.containerID()) p.addr.SetObject(id) - //nolint: contextcheck - statusError := exec.svc.get(exec.context(), p.commonPrm, withPayloadRange(rng)) + statusError := exec.svc.get(ctx, p.commonPrm, withPayloadRange(rng)) if statusError.err != nil { return nil, statusError.err diff --git a/pkg/services/object/get/container.go b/pkg/services/object/get/container.go index 882861129..cfb538d38 100644 --- a/pkg/services/object/get/container.go +++ b/pkg/services/object/get/container.go @@ -7,7 +7,7 @@ import ( "go.uber.org/zap" ) -func (exec *execCtx) executeOnContainer() { +func (exec *execCtx) executeOnContainer(ctx context.Context) { if exec.isLocal() { exec.log.Debug("return result directly") return @@ -26,7 +26,7 @@ func (exec *execCtx) executeOnContainer() { } for { - if exec.processCurrentEpoch() { + if exec.processCurrentEpoch(ctx) { break } @@ -42,7 +42,7 @@ func (exec *execCtx) executeOnContainer() { } } -func (exec *execCtx) processCurrentEpoch() bool { +func (exec *execCtx) processCurrentEpoch(ctx context.Context) bool { exec.log.Debug("process epoch", zap.Uint64("number", exec.curProcEpoch), ) @@ -52,7 +52,7 @@ func (exec *execCtx) processCurrentEpoch() bool { return true } - ctx, cancel := context.WithCancel(exec.context()) + ctx, cancel := context.WithCancel(ctx) defer cancel() exec.status = statusUndefined diff --git a/pkg/services/object/get/exec.go b/pkg/services/object/get/exec.go index 9858b32b2..2ba014574 100644 --- a/pkg/services/object/get/exec.go +++ b/pkg/services/object/get/exec.go @@ -19,12 +19,9 @@ type statusError struct { err error } -// nolint: containedctx type execCtx struct { svc *Service - ctx context.Context - prm RangePrm statusError @@ -80,10 +77,6 @@ func (exec *execCtx) setLogger(l *logger.Logger) { )} } -func (exec execCtx) context() context.Context { - return exec.ctx -} - func (exec execCtx) isLocal() bool { return exec.prm.common.LocalOnly() } @@ -217,13 +210,13 @@ func mergeSplitInfo(dst, src *objectSDK.SplitInfo) { } } -func (exec *execCtx) writeCollectedHeader() bool { +func (exec *execCtx) writeCollectedHeader(ctx context.Context) bool { if exec.ctxRange() != nil { return true } err := exec.prm.objWriter.WriteHeader( - exec.context(), + ctx, exec.collectedObject.CutPayload(), ) @@ -243,12 +236,12 @@ func (exec *execCtx) writeCollectedHeader() bool { return exec.status == statusOK } -func (exec *execCtx) writeObjectPayload(obj *objectSDK.Object) bool { +func (exec *execCtx) writeObjectPayload(ctx context.Context, obj *objectSDK.Object) bool { if exec.headOnly() { return true } - err := exec.prm.objWriter.WriteChunk(exec.context(), obj.Payload()) + err := exec.prm.objWriter.WriteChunk(ctx, obj.Payload()) switch { default: @@ -266,9 +259,9 @@ func (exec *execCtx) writeObjectPayload(obj *objectSDK.Object) bool { return err == nil } -func (exec *execCtx) writeCollectedObject() { - if ok := exec.writeCollectedHeader(); ok { - exec.writeObjectPayload(exec.collectedObject) +func (exec *execCtx) writeCollectedObject(ctx context.Context) { + if ok := exec.writeCollectedHeader(ctx); ok { + exec.writeObjectPayload(ctx, exec.collectedObject) } } diff --git a/pkg/services/object/get/get.go b/pkg/services/object/get/get.go index cdb2d96fd..0f5983e99 100644 --- a/pkg/services/object/get/get.go +++ b/pkg/services/object/get/get.go @@ -65,7 +65,6 @@ func (s *Service) Head(ctx context.Context, prm HeadPrm) error { func (s *Service) get(ctx context.Context, prm commonPrm, opts ...execOption) statusError { exec := &execCtx{ svc: s, - ctx: ctx, prm: RangePrm{ commonPrm: prm, }, @@ -78,22 +77,21 @@ func (s *Service) get(ctx context.Context, prm commonPrm, opts ...execOption) st exec.setLogger(s.log) - //nolint: contextcheck - exec.execute() + exec.execute(ctx) return exec.statusError } -func (exec *execCtx) execute() { +func (exec *execCtx) execute(ctx context.Context) { exec.log.Debug("serving request...") // perform local operation - exec.executeLocal() + exec.executeLocal(ctx) - exec.analyzeStatus(true) + exec.analyzeStatus(ctx, true) } -func (exec *execCtx) analyzeStatus(execCnr bool) { +func (exec *execCtx) analyzeStatus(ctx context.Context, execCnr bool) { // analyze local result switch exec.status { case statusOK: @@ -102,7 +100,7 @@ func (exec *execCtx) analyzeStatus(execCnr bool) { exec.log.Debug("requested object was marked as removed") case statusVIRTUAL: exec.log.Debug("requested object is virtual") - exec.assemble() + exec.assemble(ctx) case statusOutOfRange: exec.log.Debug("requested range is out of object bounds") default: @@ -111,8 +109,8 @@ func (exec *execCtx) analyzeStatus(execCnr bool) { ) if execCnr { - exec.executeOnContainer() - exec.analyzeStatus(false) + exec.executeOnContainer(ctx) + exec.analyzeStatus(ctx, false) } } } diff --git a/pkg/services/object/get/get_test.go b/pkg/services/object/get/get_test.go index 36a0e4976..3d1a95cbb 100644 --- a/pkg/services/object/get/get_test.go +++ b/pkg/services/object/get/get_test.go @@ -117,7 +117,7 @@ func newTestClient() *testClient { } } -func (c *testClient) getObject(exec *execCtx, _ client.NodeInfo) (*objectSDK.Object, error) { +func (c *testClient) getObject(ctx context.Context, exec *execCtx, _ client.NodeInfo) (*objectSDK.Object, error) { v, ok := c.results[exec.address().EncodeToString()] if !ok { var errNotFound apistatus.ObjectNotFound diff --git a/pkg/services/object/get/local.go b/pkg/services/object/get/local.go index f526af4e6..a6a77729c 100644 --- a/pkg/services/object/get/local.go +++ b/pkg/services/object/get/local.go @@ -1,6 +1,7 @@ package getsvc import ( + "context" "errors" apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" @@ -8,7 +9,7 @@ import ( "go.uber.org/zap" ) -func (exec *execCtx) executeLocal() { +func (exec *execCtx) executeLocal(ctx context.Context) { var err error exec.collectedObject, err = exec.svc.localStorage.get(exec) @@ -28,7 +29,7 @@ func (exec *execCtx) executeLocal() { case err == nil: exec.status = statusOK exec.err = nil - exec.writeCollectedObject() + exec.writeCollectedObject(ctx) case errors.As(err, &errRemoved): exec.status = statusINHUMED exec.err = errRemoved diff --git a/pkg/services/object/get/remote.go b/pkg/services/object/get/remote.go index fbfb01bcd..1532bade0 100644 --- a/pkg/services/object/get/remote.go +++ b/pkg/services/object/get/remote.go @@ -18,7 +18,7 @@ func (exec *execCtx) processNode(ctx context.Context, info client.NodeInfo) bool return true } - obj, err := client.getObject(exec, info) + obj, err := client.getObject(ctx, exec, info) var errSplitInfo *objectSDK.SplitInfoError var errRemoved *apistatus.ObjectAlreadyRemoved @@ -43,8 +43,7 @@ func (exec *execCtx) processNode(ctx context.Context, info client.NodeInfo) bool // has already been streamed to the requesting party if obj != nil { exec.collectedObject = obj - //nolint: contextcheck - exec.writeCollectedObject() + exec.writeCollectedObject(ctx) } case errors.As(err, &errRemoved): exec.status = statusINHUMED diff --git a/pkg/services/object/get/service.go b/pkg/services/object/get/service.go index e69ab4f0f..dfa3b48ac 100644 --- a/pkg/services/object/get/service.go +++ b/pkg/services/object/get/service.go @@ -1,6 +1,8 @@ package getsvc import ( + "context" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/client" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/netmap" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/engine" @@ -22,7 +24,7 @@ type Service struct { type Option func(*cfg) type getClient interface { - getObject(*execCtx, client.NodeInfo) (*object.Object, error) + getObject(context.Context, *execCtx, client.NodeInfo) (*object.Object, error) } type cfg struct { diff --git a/pkg/services/object/get/util.go b/pkg/services/object/get/util.go index d647b07f6..08c738280 100644 --- a/pkg/services/object/get/util.go +++ b/pkg/services/object/get/util.go @@ -88,7 +88,7 @@ func (c *clientCacheWrapper) get(info coreclient.NodeInfo) (getClient, error) { } // nolint: funlen -func (c *clientWrapper) getObject(exec *execCtx, info coreclient.NodeInfo) (*object.Object, error) { +func (c *clientWrapper) getObject(ctx context.Context, exec *execCtx, info coreclient.NodeInfo) (*object.Object, error) { if exec.isForwardingEnabled() { return exec.prm.forwarder(info, c.client) } @@ -101,7 +101,7 @@ func (c *clientWrapper) getObject(exec *execCtx, info coreclient.NodeInfo) (*obj if exec.headOnly() { var prm internalclient.HeadObjectPrm - prm.SetContext(exec.context()) + prm.SetContext(ctx) prm.SetClient(c.client) prm.SetTTL(exec.prm.common.TTL()) prm.SetNetmapEpoch(exec.curProcEpoch) @@ -127,7 +127,7 @@ func (c *clientWrapper) getObject(exec *execCtx, info coreclient.NodeInfo) (*obj if rng := exec.ctxRange(); rng != nil { var prm internalclient.PayloadRangePrm - prm.SetContext(exec.context()) + prm.SetContext(ctx) prm.SetClient(c.client) prm.SetTTL(exec.prm.common.TTL()) prm.SetNetmapEpoch(exec.curProcEpoch) @@ -148,7 +148,7 @@ func (c *clientWrapper) getObject(exec *execCtx, info coreclient.NodeInfo) (*obj if errors.As(err, &errAccessDenied) { // Current spec allows other storage node to deny access, // fallback to GET here. - obj, err := c.get(exec, key) + obj, err := c.get(ctx, exec, key) if err != nil { return nil, err } @@ -169,13 +169,13 @@ func (c *clientWrapper) getObject(exec *execCtx, info coreclient.NodeInfo) (*obj return payloadOnlyObject(res.PayloadRange()), nil } - return c.get(exec, key) + return c.get(ctx, exec, key) } -func (c *clientWrapper) get(exec *execCtx, key *ecdsa.PrivateKey) (*object.Object, error) { +func (c *clientWrapper) get(ctx context.Context, exec *execCtx, key *ecdsa.PrivateKey) (*object.Object, error) { var prm internalclient.GetObjectPrm - prm.SetContext(exec.context()) + prm.SetContext(ctx) prm.SetClient(c.client) prm.SetTTL(exec.prm.common.TTL()) prm.SetNetmapEpoch(exec.curProcEpoch) -- 2.45.2 From e7e63f27e8e94c8de743c7010449707825843e26 Mon Sep 17 00:00:00 2001 From: Dmitrii Stepanov Date: Fri, 31 Mar 2023 15:44:18 +0300 Subject: [PATCH 2/7] [#193] getsvc: Resolve funlen linter Signed-off-by: Dmitrii Stepanov --- pkg/services/object/get/util.go | 137 +++++++++++++++++--------------- 1 file changed, 72 insertions(+), 65 deletions(-) diff --git a/pkg/services/object/get/util.go b/pkg/services/object/get/util.go index 08c738280..a68232c9a 100644 --- a/pkg/services/object/get/util.go +++ b/pkg/services/object/get/util.go @@ -87,7 +87,6 @@ func (c *clientCacheWrapper) get(info coreclient.NodeInfo) (getClient, error) { }, nil } -// nolint: funlen func (c *clientWrapper) getObject(ctx context.Context, exec *execCtx, info coreclient.NodeInfo) (*object.Object, error) { if exec.isForwardingEnabled() { return exec.prm.forwarder(info, c.client) @@ -99,79 +98,87 @@ func (c *clientWrapper) getObject(ctx context.Context, exec *execCtx, info corec } if exec.headOnly() { - var prm internalclient.HeadObjectPrm - - prm.SetContext(ctx) - prm.SetClient(c.client) - prm.SetTTL(exec.prm.common.TTL()) - prm.SetNetmapEpoch(exec.curProcEpoch) - prm.SetAddress(exec.address()) - prm.SetPrivateKey(key) - prm.SetSessionToken(exec.prm.common.SessionToken()) - prm.SetBearerToken(exec.prm.common.BearerToken()) - prm.SetXHeaders(exec.prm.common.XHeaders()) - - if exec.isRaw() { - prm.SetRawFlag() - } - - res, err := internalclient.HeadObject(prm) - if err != nil { - return nil, err - } - - return res.Header(), nil + return c.getHeadOnly(ctx, exec, key) } // we don't specify payload writer because we accumulate // the object locally (even huge). if rng := exec.ctxRange(); rng != nil { - var prm internalclient.PayloadRangePrm - - prm.SetContext(ctx) - prm.SetClient(c.client) - prm.SetTTL(exec.prm.common.TTL()) - prm.SetNetmapEpoch(exec.curProcEpoch) - prm.SetAddress(exec.address()) - prm.SetPrivateKey(key) - prm.SetSessionToken(exec.prm.common.SessionToken()) - prm.SetBearerToken(exec.prm.common.BearerToken()) - prm.SetXHeaders(exec.prm.common.XHeaders()) - prm.SetRange(rng) - - if exec.isRaw() { - prm.SetRawFlag() - } - - res, err := internalclient.PayloadRange(prm) - if err != nil { - var errAccessDenied *apistatus.ObjectAccessDenied - if errors.As(err, &errAccessDenied) { - // Current spec allows other storage node to deny access, - // fallback to GET here. - obj, err := c.get(ctx, exec, key) - if err != nil { - return nil, err - } - - payload := obj.Payload() - from := rng.GetOffset() - to := from + rng.GetLength() - - if pLen := uint64(len(payload)); to < from || pLen < from || pLen < to { - return nil, new(apistatus.ObjectOutOfRange) - } - - return payloadOnlyObject(payload[from:to]), nil - } - return nil, err - } - - return payloadOnlyObject(res.PayloadRange()), nil + // Current spec allows other storage node to deny access, + // fallback to GET here. + return c.getRange(ctx, exec, key, rng) } return c.get(ctx, exec, key) } +func (c *clientWrapper) getRange(ctx context.Context, exec *execCtx, key *ecdsa.PrivateKey, rng *object.Range) (*object.Object, error) { + var prm internalclient.PayloadRangePrm + + prm.SetContext(ctx) + prm.SetClient(c.client) + prm.SetTTL(exec.prm.common.TTL()) + prm.SetNetmapEpoch(exec.curProcEpoch) + prm.SetAddress(exec.address()) + prm.SetPrivateKey(key) + prm.SetSessionToken(exec.prm.common.SessionToken()) + prm.SetBearerToken(exec.prm.common.BearerToken()) + prm.SetXHeaders(exec.prm.common.XHeaders()) + prm.SetRange(rng) + + if exec.isRaw() { + prm.SetRawFlag() + } + + res, err := internalclient.PayloadRange(prm) + if err != nil { + var errAccessDenied *apistatus.ObjectAccessDenied + if errors.As(err, &errAccessDenied) { + obj, err := c.get(ctx, exec, key) + if err != nil { + return nil, err + } + + payload := obj.Payload() + from := rng.GetOffset() + to := from + rng.GetLength() + + if pLen := uint64(len(payload)); to < from || pLen < from || pLen < to { + return nil, new(apistatus.ObjectOutOfRange) + } + + return payloadOnlyObject(payload[from:to]), nil + } + return nil, err + } + + return payloadOnlyObject(res.PayloadRange()), nil +} + +func (c *clientWrapper) getHeadOnly(ctx context.Context, exec *execCtx, key *ecdsa.PrivateKey) (*object.Object, error) { + var prm internalclient.HeadObjectPrm + + prm.SetContext(ctx) + prm.SetClient(c.client) + prm.SetTTL(exec.prm.common.TTL()) + prm.SetNetmapEpoch(exec.curProcEpoch) + prm.SetAddress(exec.address()) + prm.SetPrivateKey(key) + prm.SetSessionToken(exec.prm.common.SessionToken()) + prm.SetBearerToken(exec.prm.common.BearerToken()) + prm.SetXHeaders(exec.prm.common.XHeaders()) + + if exec.isRaw() { + prm.SetRawFlag() + } + + res, err := internalclient.HeadObject(prm) + if err != nil { + return nil, err + } + + return res.Header(), nil +} + func (c *clientWrapper) get(ctx context.Context, exec *execCtx, key *ecdsa.PrivateKey) (*object.Object, error) { var prm internalclient.GetObjectPrm -- 2.45.2 From e3d935fe0fe56585dc542c1aa4e8e0d4d93ecbbb Mon Sep 17 00:00:00 2001 From: Dmitrii Stepanov Date: Fri, 31 Mar 2023 16:30:46 +0300 Subject: [PATCH 3/7] [#193] getsvc: Refactor head param creation Resolve funlen linter for toHeadPrm method. Signed-off-by: Dmitrii Stepanov --- pkg/services/object/get/v2/head_forwarder.go | 180 +++++++++++++++++++ pkg/services/object/get/v2/util.go | 138 +------------- 2 files changed, 188 insertions(+), 130 deletions(-) create mode 100644 pkg/services/object/get/v2/head_forwarder.go diff --git a/pkg/services/object/get/v2/head_forwarder.go b/pkg/services/object/get/v2/head_forwarder.go new file mode 100644 index 000000000..b38da7131 --- /dev/null +++ b/pkg/services/object/get/v2/head_forwarder.go @@ -0,0 +1,180 @@ +package getsvc + +import ( + "context" + "errors" + "fmt" + "sync" + + objectV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object" + "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/refs" + "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc" + rpcclient "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc/client" + "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/session" + "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/signature" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/client" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/network" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/util" + frostfscrypto "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/crypto" + "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" + oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" +) + +type headRequestForwarder struct { + Request *objectV2.HeadRequest + Response *objectV2.HeadResponse + OnceResign *sync.Once + ObjectAddr oid.Address + KeyStorage *util.KeyStorage +} + +func (f *headRequestForwarder) forward(ctx context.Context, addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { + var err error + + key, err := f.KeyStorage.GetKey(nil) + if err != nil { + return nil, err + } + + // once compose and resign forwarding request + f.OnceResign.Do(func() { + // compose meta header of the local server + metaHdr := new(session.RequestMetaHeader) + metaHdr.SetTTL(f.Request.GetMetaHeader().GetTTL() - 1) + // TODO: #1165 think how to set the other fields + metaHdr.SetOrigin(f.Request.GetMetaHeader()) + writeCurrentVersion(metaHdr) + + f.Request.SetMetaHeader(metaHdr) + + err = signature.SignServiceMessage(key, f.Request) + }) + + if err != nil { + return nil, err + } + + headResp, err := f.sendHeadRequest(ctx, addr, c) + if err != nil { + return nil, err + } + + if err := f.verifyResponse(headResp, pubkey); err != nil { + return nil, err + } + + var ( + hdr *objectV2.Header + idSig *refs.Signature + ) + + switch v := headResp.GetBody().GetHeaderPart().(type) { + case nil: + return nil, fmt.Errorf("unexpected header type %T", v) + case *objectV2.ShortHeader: + if hdr, err = f.getHeaderFromShortHeader(v); err != nil { + return nil, err + } + case *objectV2.HeaderWithSignature: + if hdr, idSig, err = f.getHeaderAndSignature(v); err != nil { + return nil, err + } + case *objectV2.SplitInfo: + si := object.NewSplitInfoFromV2(v) + return nil, object.NewSplitInfoError(si) + } + + objv2 := new(objectV2.Object) + objv2.SetHeader(hdr) + objv2.SetSignature(idSig) + + obj := object.NewFromV2(objv2) + obj.SetID(f.ObjectAddr.Object()) + + return obj, nil +} + +func (f *headRequestForwarder) getHeaderFromShortHeader(sh *objectV2.ShortHeader) (*objectV2.Header, error) { + if !f.Request.GetBody().GetMainOnly() { + return nil, fmt.Errorf("wrong header part type: expected %T, received %T", + (*objectV2.ShortHeader)(nil), (*objectV2.HeaderWithSignature)(nil), + ) + } + + hdr := new(objectV2.Header) + hdr.SetPayloadLength(sh.GetPayloadLength()) + hdr.SetVersion(sh.GetVersion()) + hdr.SetOwnerID(sh.GetOwnerID()) + hdr.SetObjectType(sh.GetObjectType()) + hdr.SetCreationEpoch(sh.GetCreationEpoch()) + hdr.SetPayloadHash(sh.GetPayloadHash()) + hdr.SetHomomorphicHash(sh.GetHomomorphicHash()) + return hdr, nil +} + +func (f *headRequestForwarder) getHeaderAndSignature(hdrWithSig *objectV2.HeaderWithSignature) (*objectV2.Header, *refs.Signature, error) { + if f.Request.GetBody().GetMainOnly() { + return nil, nil, fmt.Errorf("wrong header part type: expected %T, received %T", + (*objectV2.HeaderWithSignature)(nil), (*objectV2.ShortHeader)(nil), + ) + } + + if hdrWithSig == nil { + return nil, nil, errors.New("nil object part") + } + + hdr := hdrWithSig.GetHeader() + idSig := hdrWithSig.GetSignature() + + if idSig == nil { + // TODO(@cthulhu-rider): #1387 use "const" error + return nil, nil, errors.New("missing signature") + } + + binID, err := f.ObjectAddr.Object().Marshal() + if err != nil { + return nil, nil, fmt.Errorf("marshal ID: %w", err) + } + + var sig frostfscrypto.Signature + if err := sig.ReadFromV2(*idSig); err != nil { + return nil, nil, fmt.Errorf("can't read signature: %w", err) + } + + if !sig.Verify(binID) { + return nil, nil, errors.New("invalid object ID signature") + } + + return hdr, idSig, nil +} + +func (f *headRequestForwarder) sendHeadRequest(ctx context.Context, addr network.Address, c client.MultiAddressClient) (*objectV2.HeadResponse, error) { + var headResp *objectV2.HeadResponse + err := c.RawForAddress(addr, func(cli *rpcclient.Client) error { + var e error + headResp, e = rpc.HeadObject(cli, f.Request, rpcclient.WithContext(ctx)) + return e + }) + if err != nil { + return nil, fmt.Errorf("sending the request failed: %w", err) + } + return headResp, nil +} + +func (f *headRequestForwarder) verifyResponse(headResp *objectV2.HeadResponse, pubkey []byte) error { + // verify response key + if err := internal.VerifyResponseKeyV2(pubkey, headResp); err != nil { + return err + } + + // verify response structure + if err := signature.VerifyServiceMessage(headResp); err != nil { + return fmt.Errorf("response verification failed: %w", err) + } + + if err := checkStatus(f.Response.GetMetaHeader().GetStatus()); err != nil { + return err + } + return nil +} diff --git a/pkg/services/object/get/v2/util.go b/pkg/services/object/get/v2/util.go index a871714a1..c659f4e7c 100644 --- a/pkg/services/object/get/v2/util.go +++ b/pkg/services/object/get/v2/util.go @@ -24,7 +24,6 @@ import ( internalclient "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal/client" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/util" apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" - frostfscrypto "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/crypto" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" versionSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/version" @@ -426,7 +425,6 @@ func (w *headResponseWriter) WriteHeader(_ context.Context, hdr *object.Object) return nil } -// nolint: funlen func (s *Service) toHeadPrm(ctx context.Context, req *objectV2.HeadRequest, resp *objectV2.HeadResponse) (*getsvc.HeadPrm, error) { body := req.GetBody() @@ -442,8 +440,6 @@ func (s *Service) toHeadPrm(ctx context.Context, req *objectV2.HeadRequest, resp return nil, fmt.Errorf("invalid object address: %w", err) } - meta := req.GetMetaHeader() - commonPrm, err := util.CommonPrmFromV2(req) if err != nil { return nil, err @@ -463,134 +459,16 @@ func (s *Service) toHeadPrm(ctx context.Context, req *objectV2.HeadRequest, resp return p, nil } - var onceResign sync.Once + forwarder := &headRequestForwarder{ + Request: req, + Response: resp, + OnceResign: &sync.Once{}, + ObjectAddr: objAddr, + KeyStorage: s.keyStorage, + } p.SetRequestForwarder(groupAddressRequestForwarder(func(addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { - var err error - - key, err := s.keyStorage.GetKey(nil) - if err != nil { - return nil, err - } - - // once compose and resign forwarding request - onceResign.Do(func() { - // compose meta header of the local server - metaHdr := new(session.RequestMetaHeader) - metaHdr.SetTTL(meta.GetTTL() - 1) - // TODO: #1165 think how to set the other fields - metaHdr.SetOrigin(meta) - writeCurrentVersion(metaHdr) - - req.SetMetaHeader(metaHdr) - - err = signature.SignServiceMessage(key, req) - }) - - if err != nil { - return nil, err - } - - // code below is copy-pasted from c.GetObjectHeader implementation, - // perhaps it is worth highlighting the utility function in frostfs-api-go - - // send Head request - var headResp *objectV2.HeadResponse - err = c.RawForAddress(addr, func(cli *rpcclient.Client) error { - headResp, err = rpc.HeadObject(cli, req, rpcclient.WithContext(ctx)) - return err - }) - if err != nil { - return nil, fmt.Errorf("sending the request failed: %w", err) - } - - // verify response key - if err = internal.VerifyResponseKeyV2(pubkey, headResp); err != nil { - return nil, err - } - - // verify response structure - if err := signature.VerifyServiceMessage(headResp); err != nil { - return nil, fmt.Errorf("response verification failed: %w", err) - } - - if err = checkStatus(resp.GetMetaHeader().GetStatus()); err != nil { - return nil, err - } - - var ( - hdr *objectV2.Header - idSig *refs.Signature - ) - - switch v := headResp.GetBody().GetHeaderPart().(type) { - case nil: - return nil, fmt.Errorf("unexpected header type %T", v) - case *objectV2.ShortHeader: - if !body.GetMainOnly() { - return nil, fmt.Errorf("wrong header part type: expected %T, received %T", - (*objectV2.ShortHeader)(nil), (*objectV2.HeaderWithSignature)(nil), - ) - } - - h := v - - hdr = new(objectV2.Header) - hdr.SetPayloadLength(h.GetPayloadLength()) - hdr.SetVersion(h.GetVersion()) - hdr.SetOwnerID(h.GetOwnerID()) - hdr.SetObjectType(h.GetObjectType()) - hdr.SetCreationEpoch(h.GetCreationEpoch()) - hdr.SetPayloadHash(h.GetPayloadHash()) - hdr.SetHomomorphicHash(h.GetHomomorphicHash()) - case *objectV2.HeaderWithSignature: - if body.GetMainOnly() { - return nil, fmt.Errorf("wrong header part type: expected %T, received %T", - (*objectV2.HeaderWithSignature)(nil), (*objectV2.ShortHeader)(nil), - ) - } - - hdrWithSig := v - if hdrWithSig == nil { - return nil, errors.New("nil object part") - } - - hdr = hdrWithSig.GetHeader() - idSig = hdrWithSig.GetSignature() - - if idSig == nil { - // TODO(@cthulhu-rider): #1387 use "const" error - return nil, errors.New("missing signature") - } - - binID, err := objAddr.Object().Marshal() - if err != nil { - return nil, fmt.Errorf("marshal ID: %w", err) - } - - var sig frostfscrypto.Signature - if err := sig.ReadFromV2(*idSig); err != nil { - return nil, fmt.Errorf("can't read signature: %w", err) - } - - if !sig.Verify(binID) { - return nil, errors.New("invalid object ID signature") - } - case *objectV2.SplitInfo: - si := object.NewSplitInfoFromV2(v) - - return nil, object.NewSplitInfoError(si) - } - - objv2 := new(objectV2.Object) - objv2.SetHeader(hdr) - objv2.SetSignature(idSig) - - obj := object.NewFromV2(objv2) - obj.SetID(objAddr.Object()) - - // convert the object - return obj, nil + return forwarder.forward(ctx, addr, c, pubkey) })) return p, nil -- 2.45.2 From 7f7d7555f184e33de4e59884bdd631771feb9be8 Mon Sep 17 00:00:00 2001 From: Dmitrii Stepanov Date: Fri, 31 Mar 2023 17:38:38 +0300 Subject: [PATCH 4/7] [#193] getsvc: Refactor get params creation Resolve funlen linter for toPrm function. Signed-off-by: Dmitrii Stepanov --- pkg/services/object/get/v2/get_forwarder.go | 171 ++++++++++++++++++++ pkg/services/object/get/v2/util.go | 138 ++-------------- 2 files changed, 180 insertions(+), 129 deletions(-) create mode 100644 pkg/services/object/get/v2/get_forwarder.go diff --git a/pkg/services/object/get/v2/get_forwarder.go b/pkg/services/object/get/v2/get_forwarder.go new file mode 100644 index 000000000..b0ba47523 --- /dev/null +++ b/pkg/services/object/get/v2/get_forwarder.go @@ -0,0 +1,171 @@ +package getsvc + +import ( + "context" + "errors" + "fmt" + "io" + "sync" + + objectV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object" + "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc" + rpcclient "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc/client" + "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/session" + "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/signature" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/client" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/network" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal" + internalclient "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal/client" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/util" + "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" +) + +type getRequestForwarder struct { + OnceResign *sync.Once + OnceHeaderSending *sync.Once + GlobalProgress int + KeyStorage *util.KeyStorage + Request *objectV2.GetRequest + Stream *streamObjectWriter +} + +func (f *getRequestForwarder) forward(ctx context.Context, addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { + key, err := f.KeyStorage.GetKey(nil) + if err != nil { + return nil, err + } + + // once compose and resign forwarding request + f.OnceResign.Do(func() { + // compose meta header of the local server + metaHdr := new(session.RequestMetaHeader) + metaHdr.SetTTL(f.Request.GetMetaHeader().GetTTL() - 1) + // TODO: #1165 think how to set the other fields + metaHdr.SetOrigin(f.Request.GetMetaHeader()) + writeCurrentVersion(metaHdr) + f.Request.SetMetaHeader(metaHdr) + err = signature.SignServiceMessage(key, f.Request) + }) + + if err != nil { + return nil, err + } + + getStream, err := f.openStream(ctx, addr, c) + if err != nil { + return nil, err + } + return nil, f.readStream(ctx, c, getStream, pubkey) +} + +func (f *getRequestForwarder) verifyResponse(resp *objectV2.GetResponse, pubkey []byte) error { + // verify response key + if err := internal.VerifyResponseKeyV2(pubkey, resp); err != nil { + return err + } + + // verify response structure + if err := signature.VerifyServiceMessage(resp); err != nil { + return fmt.Errorf("response verification failed: %w", err) + } + + if err := checkStatus(resp.GetMetaHeader().GetStatus()); err != nil { + return err + } + return nil +} + +func (f *getRequestForwarder) writeHeader(ctx context.Context, v *objectV2.GetObjectPartInit) error { + obj := new(objectV2.Object) + + obj.SetObjectID(v.GetObjectID()) + obj.SetSignature(v.GetSignature()) + obj.SetHeader(v.GetHeader()) + + var err error + f.OnceHeaderSending.Do(func() { + err = f.Stream.WriteHeader(ctx, object.NewFromV2(obj)) + }) + if err != nil { + return fmt.Errorf("could not write object header in Get forwarder: %w", err) + } + return nil +} + +func (f *getRequestForwarder) openStream(ctx context.Context, addr network.Address, c client.MultiAddressClient) (*rpc.GetResponseReader, error) { + var getStream *rpc.GetResponseReader + err := c.RawForAddress(addr, func(cli *rpcclient.Client) error { + var e error + getStream, e = rpc.GetObject(cli, f.Request, rpcclient.WithContext(ctx)) + return e + }) + if err != nil { + return nil, fmt.Errorf("stream opening failed: %w", err) + } + return getStream, nil +} + +func (f *getRequestForwarder) readStream(ctx context.Context, c client.MultiAddressClient, getStream *rpc.GetResponseReader, pubkey []byte) error { + var ( + headWas bool + resp = new(objectV2.GetResponse) + localProgress int + ) + + for { + // receive message from server stream + err := getStream.Read(resp) + if err != nil { + if errors.Is(err, io.EOF) { + if !headWas { + return io.ErrUnexpectedEOF + } + + break + } + + internalclient.ReportError(c, err) + return fmt.Errorf("reading the response failed: %w", err) + } + + if err := f.verifyResponse(resp, pubkey); err != nil { + return err + } + + switch v := resp.GetBody().GetObjectPart().(type) { + default: + return fmt.Errorf("unexpected object part %T", v) + case *objectV2.GetObjectPartInit: + if headWas { + return errWrongMessageSeq + } + headWas = true + if err := f.writeHeader(ctx, v); err != nil { + return err + } + case *objectV2.GetObjectPartChunk: + if !headWas { + return errWrongMessageSeq + } + + origChunk := v.GetChunk() + + chunk := chunkToSend(f.GlobalProgress, localProgress, origChunk) + if len(chunk) == 0 { + localProgress += len(origChunk) + continue + } + + if err = f.Stream.WriteChunk(ctx, chunk); err != nil { + return fmt.Errorf("could not write object chunk in Get forwarder: %w", err) + } + + localProgress += len(origChunk) + f.GlobalProgress += len(chunk) + case *objectV2.SplitInfo: + si := object.NewSplitInfoFromV2(v) + return object.NewSplitInfoError(si) + } + } + return nil +} diff --git a/pkg/services/object/get/v2/util.go b/pkg/services/object/get/v2/util.go index c659f4e7c..3a9cf3b07 100644 --- a/pkg/services/object/get/v2/util.go +++ b/pkg/services/object/get/v2/util.go @@ -32,7 +32,6 @@ import ( var errWrongMessageSeq = errors.New("incorrect message sequence") -// nolint: funlen, gocognit func (s *Service) toPrm(req *objectV2.GetRequest, stream objectSvc.GetObjectStream) (*getsvc.Prm, error) { body := req.GetBody() @@ -48,8 +47,6 @@ func (s *Service) toPrm(req *objectV2.GetRequest, stream objectSvc.GetObjectStre return nil, fmt.Errorf("invalid object address: %w", err) } - meta := req.GetMetaHeader() - commonPrm, err := util.CommonPrmFromV2(req) if err != nil { return nil, err @@ -65,134 +62,17 @@ func (s *Service) toPrm(req *objectV2.GetRequest, stream objectSvc.GetObjectStre p.SetObjectWriter(streamWrapper) if !commonPrm.LocalOnly() { - var onceResign sync.Once - - var onceHeaderSending sync.Once - var globalProgress int + forwarder := &getRequestForwarder{ + OnceResign: &sync.Once{}, + OnceHeaderSending: &sync.Once{}, + GlobalProgress: 0, + KeyStorage: s.keyStorage, + Request: req, + Stream: streamWrapper, + } p.SetRequestForwarder(groupAddressRequestForwarder(func(addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { - var err error - - key, err := s.keyStorage.GetKey(nil) - if err != nil { - return nil, err - } - - // once compose and resign forwarding request - onceResign.Do(func() { - // compose meta header of the local server - metaHdr := new(session.RequestMetaHeader) - metaHdr.SetTTL(meta.GetTTL() - 1) - // TODO: #1165 think how to set the other fields - metaHdr.SetOrigin(meta) - writeCurrentVersion(metaHdr) - - req.SetMetaHeader(metaHdr) - - err = signature.SignServiceMessage(key, req) - }) - - if err != nil { - return nil, err - } - - // code below is copy-pasted from c.GetObject implementation, - // perhaps it is worth highlighting the utility function in frostfs-api-go - - // open stream - var getStream *rpc.GetResponseReader - err = c.RawForAddress(addr, func(cli *rpcclient.Client) error { - getStream, err = rpc.GetObject(cli, req, rpcclient.WithContext(stream.Context())) - return err - }) - if err != nil { - return nil, fmt.Errorf("stream opening failed: %w", err) - } - - var ( - headWas bool - resp = new(objectV2.GetResponse) - localProgress int - ) - - for { - // receive message from server stream - err := getStream.Read(resp) - if err != nil { - if errors.Is(err, io.EOF) { - if !headWas { - return nil, io.ErrUnexpectedEOF - } - - break - } - - internalclient.ReportError(c, err) - return nil, fmt.Errorf("reading the response failed: %w", err) - } - - // verify response key - if err = internal.VerifyResponseKeyV2(pubkey, resp); err != nil { - return nil, err - } - - // verify response structure - if err := signature.VerifyServiceMessage(resp); err != nil { - return nil, fmt.Errorf("response verification failed: %w", err) - } - - if err = checkStatus(resp.GetMetaHeader().GetStatus()); err != nil { - return nil, err - } - - switch v := resp.GetBody().GetObjectPart().(type) { - default: - return nil, fmt.Errorf("unexpected object part %T", v) - case *objectV2.GetObjectPartInit: - if headWas { - return nil, errWrongMessageSeq - } - - headWas = true - - obj := new(objectV2.Object) - - obj.SetObjectID(v.GetObjectID()) - obj.SetSignature(v.GetSignature()) - obj.SetHeader(v.GetHeader()) - - onceHeaderSending.Do(func() { - err = streamWrapper.WriteHeader(stream.Context(), object.NewFromV2(obj)) - }) - if err != nil { - return nil, fmt.Errorf("could not write object header in Get forwarder: %w", err) - } - case *objectV2.GetObjectPartChunk: - if !headWas { - return nil, errWrongMessageSeq - } - - origChunk := v.GetChunk() - - chunk := chunkToSend(globalProgress, localProgress, origChunk) - if len(chunk) == 0 { - localProgress += len(origChunk) - continue - } - - if err = streamWrapper.WriteChunk(stream.Context(), chunk); err != nil { - return nil, fmt.Errorf("could not write object chunk in Get forwarder: %w", err) - } - - localProgress += len(origChunk) - globalProgress += len(chunk) - case *objectV2.SplitInfo: - si := object.NewSplitInfoFromV2(v) - return nil, object.NewSplitInfoError(si) - } - } - - return nil, nil + return forwarder.forward(stream.Context(), addr, c, pubkey) })) } -- 2.45.2 From c32cd6265524208338319209603b86484fad5012 Mon Sep 17 00:00:00 2001 From: Dmitrii Stepanov Date: Fri, 31 Mar 2023 17:41:42 +0300 Subject: [PATCH 5/7] [#193] getsvc: Refactor get range params creation Resolve funlen linter for toRangePrm function. Signed-off-by: Dmitrii Stepanov --- .../object/get/v2/get_range_forwarder.go | 137 ++++++++++++++++++ pkg/services/object/get/v2/util.go | 110 +------------- 2 files changed, 144 insertions(+), 103 deletions(-) create mode 100644 pkg/services/object/get/v2/get_range_forwarder.go diff --git a/pkg/services/object/get/v2/get_range_forwarder.go b/pkg/services/object/get/v2/get_range_forwarder.go new file mode 100644 index 000000000..a9526f714 --- /dev/null +++ b/pkg/services/object/get/v2/get_range_forwarder.go @@ -0,0 +1,137 @@ +package getsvc + +import ( + "context" + "errors" + "fmt" + "io" + "sync" + + objectV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object" + "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc" + rpcclient "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc/client" + "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/session" + "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/signature" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/client" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/network" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal" + internalclient "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal/client" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/util" + "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" +) + +type getRangeRequestForwarder struct { + OnceResign *sync.Once + GlobalProgress int + KeyStorage *util.KeyStorage + Request *objectV2.GetRangeRequest + Stream *streamObjectRangeWriter +} + +func (f *getRangeRequestForwarder) forward(ctx context.Context, addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { + key, err := f.KeyStorage.GetKey(nil) + if err != nil { + return nil, err + } + + // once compose and resign forwarding request + f.OnceResign.Do(func() { + // compose meta header of the local server + metaHdr := new(session.RequestMetaHeader) + metaHdr.SetTTL(f.Request.GetMetaHeader().GetTTL() - 1) + // TODO: #1165 think how to set the other fields + metaHdr.SetOrigin(f.Request.GetMetaHeader()) + writeCurrentVersion(metaHdr) + + f.Request.SetMetaHeader(metaHdr) + + err = signature.SignServiceMessage(key, f.Request) + }) + + if err != nil { + return nil, err + } + + rangeStream, err := f.openStream(ctx, addr, c) + if err != nil { + return nil, err + } + + return nil, f.readStream(ctx, rangeStream, c, pubkey) +} + +func (f *getRangeRequestForwarder) verifyResponse(resp *objectV2.GetRangeResponse, pubkey []byte) error { + // verify response key + if err := internal.VerifyResponseKeyV2(pubkey, resp); err != nil { + return err + } + + // verify response structure + if err := signature.VerifyServiceMessage(resp); err != nil { + return fmt.Errorf("could not verify %T: %w", resp, err) + } + + if err := checkStatus(resp.GetMetaHeader().GetStatus()); err != nil { + return err + } + return nil +} + +func (f *getRangeRequestForwarder) openStream(ctx context.Context, addr network.Address, c client.MultiAddressClient) (*rpc.ObjectRangeResponseReader, error) { + // open stream + var rangeStream *rpc.ObjectRangeResponseReader + err := c.RawForAddress(addr, func(cli *rpcclient.Client) error { + var e error + rangeStream, e = rpc.GetObjectRange(cli, f.Request, rpcclient.WithContext(ctx)) + return e + }) + if err != nil { + return nil, fmt.Errorf("could not create Get payload range stream: %w", err) + } + return rangeStream, nil +} + +func (f *getRangeRequestForwarder) readStream(ctx context.Context, rangeStream *rpc.ObjectRangeResponseReader, c client.MultiAddressClient, pubkey []byte) error { + resp := new(objectV2.GetRangeResponse) + var localProgress int + + for { + // receive message from server stream + err := rangeStream.Read(resp) + if err != nil { + if errors.Is(err, io.EOF) { + break + } + internalclient.ReportError(c, err) + return fmt.Errorf("reading the response failed: %w", err) + } + + if err := f.verifyResponse(resp, pubkey); err != nil { + return err + } + + switch v := resp.GetBody().GetRangePart().(type) { + case nil: + return fmt.Errorf("unexpected range type %T", v) + case *objectV2.GetRangePartChunk: + origChunk := v.GetChunk() + + chunk := chunkToSend(f.GlobalProgress, localProgress, origChunk) + if len(chunk) == 0 { + localProgress += len(origChunk) + continue + } + + if err = f.Stream.WriteChunk(ctx, chunk); err != nil { + return fmt.Errorf("could not write object chunk in GetRange forwarder: %w", err) + } + + localProgress += len(origChunk) + f.GlobalProgress += len(chunk) + case *objectV2.SplitInfo: + si := object.NewSplitInfoFromV2(v) + return object.NewSplitInfoError(si) + } + } + return nil +} diff --git a/pkg/services/object/get/v2/util.go b/pkg/services/object/get/v2/util.go index 3a9cf3b07..dffa0d9b1 100644 --- a/pkg/services/object/get/v2/util.go +++ b/pkg/services/object/get/v2/util.go @@ -6,22 +6,16 @@ import ( "errors" "fmt" "hash" - "io" "sync" objectV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object" "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/refs" - "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc" - rpcclient "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc/client" "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/session" - "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/signature" "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/status" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/client" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/network" objectSvc "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object" getsvc "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/get" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal" - internalclient "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal/client" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/util" apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" @@ -79,7 +73,6 @@ func (s *Service) toPrm(req *objectV2.GetRequest, stream objectSvc.GetObjectStre return p, nil } -// nolint: funlen, gocognit func (s *Service) toRangePrm(req *objectV2.GetRangeRequest, stream objectSvc.GetObjectRangeStream) (*getsvc.RangePrm, error) { body := req.GetBody() @@ -95,8 +88,6 @@ func (s *Service) toRangePrm(req *objectV2.GetRangeRequest, stream objectSvc.Get return nil, fmt.Errorf("invalid object address: %w", err) } - meta := req.GetMetaHeader() - commonPrm, err := util.CommonPrmFromV2(req) if err != nil { return nil, err @@ -118,103 +109,16 @@ func (s *Service) toRangePrm(req *objectV2.GetRangeRequest, stream objectSvc.Get } if !commonPrm.LocalOnly() { - var onceResign sync.Once - var globalProgress int - - key, err := s.keyStorage.GetKey(nil) - if err != nil { - return nil, err + forwarder := &getRangeRequestForwarder{ + OnceResign: &sync.Once{}, + GlobalProgress: 0, + KeyStorage: s.keyStorage, + Request: req, + Stream: streamWrapper, } p.SetRequestForwarder(groupAddressRequestForwarder(func(addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { - var err error - - // once compose and resign forwarding request - onceResign.Do(func() { - // compose meta header of the local server - metaHdr := new(session.RequestMetaHeader) - metaHdr.SetTTL(meta.GetTTL() - 1) - // TODO: #1165 think how to set the other fields - metaHdr.SetOrigin(meta) - writeCurrentVersion(metaHdr) - - req.SetMetaHeader(metaHdr) - - err = signature.SignServiceMessage(key, req) - }) - - if err != nil { - return nil, err - } - - // code below is copy-pasted from c.ObjectPayloadRangeData implementation, - // perhaps it is worth highlighting the utility function in frostfs-api-go - - // open stream - var rangeStream *rpc.ObjectRangeResponseReader - err = c.RawForAddress(addr, func(cli *rpcclient.Client) error { - rangeStream, err = rpc.GetObjectRange(cli, req, rpcclient.WithContext(stream.Context())) - return err - }) - if err != nil { - return nil, fmt.Errorf("could not create Get payload range stream: %w", err) - } - - resp := new(objectV2.GetRangeResponse) - var localProgress int - - for { - // receive message from server stream - err := rangeStream.Read(resp) - if err != nil { - if errors.Is(err, io.EOF) { - break - } - - internalclient.ReportError(c, err) - return nil, fmt.Errorf("reading the response failed: %w", err) - } - - // verify response key - if err = internal.VerifyResponseKeyV2(pubkey, resp); err != nil { - return nil, err - } - - // verify response structure - if err := signature.VerifyServiceMessage(resp); err != nil { - return nil, fmt.Errorf("could not verify %T: %w", resp, err) - } - - if err = checkStatus(resp.GetMetaHeader().GetStatus()); err != nil { - return nil, err - } - - switch v := resp.GetBody().GetRangePart().(type) { - case nil: - return nil, fmt.Errorf("unexpected range type %T", v) - case *objectV2.GetRangePartChunk: - origChunk := v.GetChunk() - - chunk := chunkToSend(globalProgress, localProgress, origChunk) - if len(chunk) == 0 { - localProgress += len(origChunk) - continue - } - - if err = streamWrapper.WriteChunk(stream.Context(), chunk); err != nil { - return nil, fmt.Errorf("could not write object chunk in GetRange forwarder: %w", err) - } - - localProgress += len(origChunk) - globalProgress += len(chunk) - case *objectV2.SplitInfo: - si := object.NewSplitInfoFromV2(v) - - return nil, object.NewSplitInfoError(si) - } - } - - return nil, nil + return forwarder.forward(stream.Context(), addr, c, pubkey) })) } -- 2.45.2 From bc26c3c6aca7f3ba0a8724479f7cb1bde4fc3328 Mon Sep 17 00:00:00 2001 From: Dmitrii Stepanov Date: Mon, 3 Apr 2023 11:17:59 +0300 Subject: [PATCH 6/7] [#193] getsvc: Edit request forwarder signature Pass context to forwarder direct, without closure. Signed-off-by: Dmitrii Stepanov --- pkg/services/object/get/prm.go | 2 +- pkg/services/object/get/util.go | 2 +- pkg/services/object/get/v2/get_forwarder.go | 2 +- .../object/get/v2/get_range_forwarder.go | 2 +- pkg/services/object/get/v2/head_forwarder.go | 2 +- pkg/services/object/get/v2/util.go | 18 ++++++------------ 6 files changed, 11 insertions(+), 17 deletions(-) diff --git a/pkg/services/object/get/prm.go b/pkg/services/object/get/prm.go index 88848264e..7a0f1e062 100644 --- a/pkg/services/object/get/prm.go +++ b/pkg/services/object/get/prm.go @@ -59,7 +59,7 @@ type RangeHashPrm struct { salt []byte } -type RequestForwarder func(coreclient.NodeInfo, coreclient.MultiAddressClient) (*object.Object, error) +type RequestForwarder func(context.Context, coreclient.NodeInfo, coreclient.MultiAddressClient) (*object.Object, error) // HeadPrm groups parameters of Head service call. type HeadPrm struct { diff --git a/pkg/services/object/get/util.go b/pkg/services/object/get/util.go index a68232c9a..7986d05c0 100644 --- a/pkg/services/object/get/util.go +++ b/pkg/services/object/get/util.go @@ -89,7 +89,7 @@ func (c *clientCacheWrapper) get(info coreclient.NodeInfo) (getClient, error) { func (c *clientWrapper) getObject(ctx context.Context, exec *execCtx, info coreclient.NodeInfo) (*object.Object, error) { if exec.isForwardingEnabled() { - return exec.prm.forwarder(info, c.client) + return exec.prm.forwarder(ctx, info, c.client) } key, err := exec.key() diff --git a/pkg/services/object/get/v2/get_forwarder.go b/pkg/services/object/get/v2/get_forwarder.go index b0ba47523..7314cceb5 100644 --- a/pkg/services/object/get/v2/get_forwarder.go +++ b/pkg/services/object/get/v2/get_forwarder.go @@ -29,7 +29,7 @@ type getRequestForwarder struct { Stream *streamObjectWriter } -func (f *getRequestForwarder) forward(ctx context.Context, addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { +func (f *getRequestForwarder) forwardRequestToNode(ctx context.Context, addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { key, err := f.KeyStorage.GetKey(nil) if err != nil { return nil, err diff --git a/pkg/services/object/get/v2/get_range_forwarder.go b/pkg/services/object/get/v2/get_range_forwarder.go index a9526f714..8fa4351d4 100644 --- a/pkg/services/object/get/v2/get_range_forwarder.go +++ b/pkg/services/object/get/v2/get_range_forwarder.go @@ -28,7 +28,7 @@ type getRangeRequestForwarder struct { Stream *streamObjectRangeWriter } -func (f *getRangeRequestForwarder) forward(ctx context.Context, addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { +func (f *getRangeRequestForwarder) forwardRequestToNode(ctx context.Context, addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { key, err := f.KeyStorage.GetKey(nil) if err != nil { return nil, err diff --git a/pkg/services/object/get/v2/head_forwarder.go b/pkg/services/object/get/v2/head_forwarder.go index b38da7131..e0b58a35d 100644 --- a/pkg/services/object/get/v2/head_forwarder.go +++ b/pkg/services/object/get/v2/head_forwarder.go @@ -29,7 +29,7 @@ type headRequestForwarder struct { KeyStorage *util.KeyStorage } -func (f *headRequestForwarder) forward(ctx context.Context, addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { +func (f *headRequestForwarder) forwardRequestToNode(ctx context.Context, addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { var err error key, err := f.KeyStorage.GetKey(nil) diff --git a/pkg/services/object/get/v2/util.go b/pkg/services/object/get/v2/util.go index dffa0d9b1..75228e2b1 100644 --- a/pkg/services/object/get/v2/util.go +++ b/pkg/services/object/get/v2/util.go @@ -65,9 +65,7 @@ func (s *Service) toPrm(req *objectV2.GetRequest, stream objectSvc.GetObjectStre Stream: streamWrapper, } - p.SetRequestForwarder(groupAddressRequestForwarder(func(addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { - return forwarder.forward(stream.Context(), addr, c, pubkey) - })) + p.SetRequestForwarder(groupAddressRequestForwarder(forwarder.forwardRequestToNode)) } return p, nil @@ -117,9 +115,7 @@ func (s *Service) toRangePrm(req *objectV2.GetRangeRequest, stream objectSvc.Get Stream: streamWrapper, } - p.SetRequestForwarder(groupAddressRequestForwarder(func(addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { - return forwarder.forward(stream.Context(), addr, c, pubkey) - })) + p.SetRequestForwarder(groupAddressRequestForwarder(forwarder.forwardRequestToNode)) } return p, nil @@ -251,9 +247,7 @@ func (s *Service) toHeadPrm(ctx context.Context, req *objectV2.HeadRequest, resp KeyStorage: s.keyStorage, } - p.SetRequestForwarder(groupAddressRequestForwarder(func(addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { - return forwarder.forward(ctx, addr, c, pubkey) - })) + p.SetRequestForwarder(groupAddressRequestForwarder(forwarder.forwardRequestToNode)) return p, nil } @@ -321,8 +315,8 @@ func toShortObjectHeader(hdr *object.Object) objectV2.GetHeaderPart { return sh } -func groupAddressRequestForwarder(f func(network.Address, client.MultiAddressClient, []byte) (*object.Object, error)) getsvc.RequestForwarder { - return func(info client.NodeInfo, c client.MultiAddressClient) (*object.Object, error) { +func groupAddressRequestForwarder(f func(context.Context, network.Address, client.MultiAddressClient, []byte) (*object.Object, error)) getsvc.RequestForwarder { + return func(ctx context.Context, info client.NodeInfo, c client.MultiAddressClient) (*object.Object, error) { var ( firstErr error res *object.Object @@ -343,7 +337,7 @@ func groupAddressRequestForwarder(f func(network.Address, client.MultiAddressCli // would be nice to log otherwise }() - res, err = f(addr, c, key) + res, err = f(ctx, addr, c, key) return }) -- 2.45.2 From 7dbeb09b9435809948e2afed81048aec9306a822 Mon Sep 17 00:00:00 2001 From: Dmitrii Stepanov Date: Tue, 4 Apr 2023 13:20:57 +0300 Subject: [PATCH 7/7] [#193] getsvc: Reduce private key requests Get private key only once for request forwaring. Signed-off-by: Dmitrii Stepanov --- pkg/services/object/get/v2/get_forwarder.go | 11 ++++------ .../object/get/v2/get_range_forwarder.go | 11 ++++------ pkg/services/object/get/v2/head_forwarder.go | 11 +++------- pkg/services/object/get/v2/util.go | 21 ++++++++++++++++--- 4 files changed, 29 insertions(+), 25 deletions(-) diff --git a/pkg/services/object/get/v2/get_forwarder.go b/pkg/services/object/get/v2/get_forwarder.go index 7314cceb5..330a0642f 100644 --- a/pkg/services/object/get/v2/get_forwarder.go +++ b/pkg/services/object/get/v2/get_forwarder.go @@ -2,6 +2,7 @@ package getsvc import ( "context" + "crypto/ecdsa" "errors" "fmt" "io" @@ -16,7 +17,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/network" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal" internalclient "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal/client" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/util" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" ) @@ -24,16 +24,13 @@ type getRequestForwarder struct { OnceResign *sync.Once OnceHeaderSending *sync.Once GlobalProgress int - KeyStorage *util.KeyStorage + Key *ecdsa.PrivateKey Request *objectV2.GetRequest Stream *streamObjectWriter } func (f *getRequestForwarder) forwardRequestToNode(ctx context.Context, addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { - key, err := f.KeyStorage.GetKey(nil) - if err != nil { - return nil, err - } + var err error // once compose and resign forwarding request f.OnceResign.Do(func() { @@ -44,7 +41,7 @@ func (f *getRequestForwarder) forwardRequestToNode(ctx context.Context, addr net metaHdr.SetOrigin(f.Request.GetMetaHeader()) writeCurrentVersion(metaHdr) f.Request.SetMetaHeader(metaHdr) - err = signature.SignServiceMessage(key, f.Request) + err = signature.SignServiceMessage(f.Key, f.Request) }) if err != nil { diff --git a/pkg/services/object/get/v2/get_range_forwarder.go b/pkg/services/object/get/v2/get_range_forwarder.go index 8fa4351d4..5893f8de3 100644 --- a/pkg/services/object/get/v2/get_range_forwarder.go +++ b/pkg/services/object/get/v2/get_range_forwarder.go @@ -2,6 +2,7 @@ package getsvc import ( "context" + "crypto/ecdsa" "errors" "fmt" "io" @@ -16,23 +17,19 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/network" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal" internalclient "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal/client" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/util" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" ) type getRangeRequestForwarder struct { OnceResign *sync.Once GlobalProgress int - KeyStorage *util.KeyStorage + Key *ecdsa.PrivateKey Request *objectV2.GetRangeRequest Stream *streamObjectRangeWriter } func (f *getRangeRequestForwarder) forwardRequestToNode(ctx context.Context, addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { - key, err := f.KeyStorage.GetKey(nil) - if err != nil { - return nil, err - } + var err error // once compose and resign forwarding request f.OnceResign.Do(func() { @@ -45,7 +42,7 @@ func (f *getRangeRequestForwarder) forwardRequestToNode(ctx context.Context, add f.Request.SetMetaHeader(metaHdr) - err = signature.SignServiceMessage(key, f.Request) + err = signature.SignServiceMessage(f.Key, f.Request) }) if err != nil { diff --git a/pkg/services/object/get/v2/head_forwarder.go b/pkg/services/object/get/v2/head_forwarder.go index e0b58a35d..45c0174fd 100644 --- a/pkg/services/object/get/v2/head_forwarder.go +++ b/pkg/services/object/get/v2/head_forwarder.go @@ -2,6 +2,7 @@ package getsvc import ( "context" + "crypto/ecdsa" "errors" "fmt" "sync" @@ -15,7 +16,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/client" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/network" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/util" frostfscrypto "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/crypto" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" @@ -26,17 +26,12 @@ type headRequestForwarder struct { Response *objectV2.HeadResponse OnceResign *sync.Once ObjectAddr oid.Address - KeyStorage *util.KeyStorage + Key *ecdsa.PrivateKey } func (f *headRequestForwarder) forwardRequestToNode(ctx context.Context, addr network.Address, c client.MultiAddressClient, pubkey []byte) (*object.Object, error) { var err error - key, err := f.KeyStorage.GetKey(nil) - if err != nil { - return nil, err - } - // once compose and resign forwarding request f.OnceResign.Do(func() { // compose meta header of the local server @@ -48,7 +43,7 @@ func (f *headRequestForwarder) forwardRequestToNode(ctx context.Context, addr ne f.Request.SetMetaHeader(metaHdr) - err = signature.SignServiceMessage(key, f.Request) + err = signature.SignServiceMessage(f.Key, f.Request) }) if err != nil { diff --git a/pkg/services/object/get/v2/util.go b/pkg/services/object/get/v2/util.go index 75228e2b1..3a50a6ca5 100644 --- a/pkg/services/object/get/v2/util.go +++ b/pkg/services/object/get/v2/util.go @@ -56,11 +56,16 @@ func (s *Service) toPrm(req *objectV2.GetRequest, stream objectSvc.GetObjectStre p.SetObjectWriter(streamWrapper) if !commonPrm.LocalOnly() { + key, err := s.keyStorage.GetKey(nil) + if err != nil { + return nil, err + } + forwarder := &getRequestForwarder{ OnceResign: &sync.Once{}, OnceHeaderSending: &sync.Once{}, GlobalProgress: 0, - KeyStorage: s.keyStorage, + Key: key, Request: req, Stream: streamWrapper, } @@ -107,10 +112,15 @@ func (s *Service) toRangePrm(req *objectV2.GetRangeRequest, stream objectSvc.Get } if !commonPrm.LocalOnly() { + key, err := s.keyStorage.GetKey(nil) + if err != nil { + return nil, err + } + forwarder := &getRangeRequestForwarder{ OnceResign: &sync.Once{}, GlobalProgress: 0, - KeyStorage: s.keyStorage, + Key: key, Request: req, Stream: streamWrapper, } @@ -239,12 +249,17 @@ func (s *Service) toHeadPrm(ctx context.Context, req *objectV2.HeadRequest, resp return p, nil } + key, err := s.keyStorage.GetKey(nil) + if err != nil { + return nil, err + } + forwarder := &headRequestForwarder{ Request: req, Response: resp, OnceResign: &sync.Once{}, ObjectAddr: objAddr, - KeyStorage: s.keyStorage, + Key: key, } p.SetRequestForwarder(groupAddressRequestForwarder(forwarder.forwardRequestToNode)) -- 2.45.2