Skip to content

Commit

Permalink
node: Return status responses only
Browse files Browse the repository at this point in the history
NeoFS statuses were introduced a long time ago. Until now, the server
supported responses to "old" clients with gRPC statuses. However, now
this approach is considered incorrect since it does not allow
integrating applications with the NeoFS behavioral model.

Refs #961.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
  • Loading branch information
cthulhu-rider committed May 6, 2024
1 parent 561ea49 commit 775f168
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 65 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Changelog for NeoFS Node
### Fixed

### Changed
- SN API server responds with status message even to old clients from now (#2846)

### Removed
- IR contracts deployment code. Moved to the contracts repo (#2812)
Expand Down
78 changes: 13 additions & 65 deletions pkg/services/util/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ type RequestMessageStreamer struct {

respCons ResponseConstructor

statusSupported bool

sendErr error
}

Expand All @@ -62,9 +60,6 @@ func NewUnarySignService(key *ecdsa.PrivateKey) *SignService {
}

func (s *RequestMessageStreamer) Send(req any) error {
// req argument should be strengthen with type RequestMessage
s.statusSupported = isStatusSupported(req.(RequestMessage)) // panic is OK here for now

var err error

// verify request signatures
Expand All @@ -75,10 +70,6 @@ func (s *RequestMessageStreamer) Send(req any) error {
}

if err != nil {
if !s.statusSupported {
return err
}

s.sendErr = err

return ErrAbortStream
Expand All @@ -103,17 +94,15 @@ func (s *RequestMessageStreamer) CloseAndRecv() (ResponseMessage, error) {
}

if err != nil {
if !s.statusSupported {
return nil, err
}

resp = s.respCons()

setStatusV2(resp, err)
}

if err = signResponse(s.key, resp, s.statusSupported); err != nil {
return nil, err
if err = signature.SignServiceMessage(s.key, resp); err != nil {

Check warning on line 102 in pkg/services/util/sign.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/util/sign.go#L102

Added line #L102 was not covered by tests
// We can't pass this error as status code since response will be unsigned.
// Isn't expected in practice, so panic is ok here.
panic(err)

Check warning on line 105 in pkg/services/util/sign.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/util/sign.go#L105

Added line #L105 was not covered by tests
}

return resp, nil
Expand All @@ -135,36 +124,29 @@ func (s *SignService) HandleServerStreamRequest(
blankResp ResponseConstructor,
respWriterCaller func(ResponseMessageWriter) error,
) error {
// handle protocol versions <=2.10 (API statuses was introduced in 2.11 only)

// req argument should be strengthen with type RequestMessage
statusSupported := isStatusSupported(req.(RequestMessage)) // panic is OK here for now

var err error

// verify request signatures
if err = signature.VerifyServiceMessage(req); err != nil {
err = fmt.Errorf("could not verify request: %w", err)
} else {
err = respWriterCaller(func(resp ResponseMessage) error {
if err := signResponse(s.key, resp, statusSupported); err != nil {
return err
if err := signature.SignServiceMessage(s.key, resp); err != nil {

Check warning on line 134 in pkg/services/util/sign.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/util/sign.go#L134

Added line #L134 was not covered by tests
// We can't pass this error as status code since response will be unsigned.
// Isn't expected in practice, so panic is ok here.
panic(err)

Check warning on line 137 in pkg/services/util/sign.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/util/sign.go#L137

Added line #L137 was not covered by tests
}

return respWriter(resp)
})
}

if err != nil {
if !statusSupported {
return err
}

resp := blankResp()

setStatusV2(resp, err)

_ = signResponse(s.key, resp, false) // panics or returns nil with false arg
_ = signature.SignServiceMessage(s.key, resp)

Check warning on line 149 in pkg/services/util/sign.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/util/sign.go#L149

Added line #L149 was not covered by tests

return respWriter(resp)
}
Expand All @@ -173,11 +155,6 @@ func (s *SignService) HandleServerStreamRequest(
}

func (s *SignService) HandleUnaryRequest(ctx context.Context, req any, handler UnaryHandler, blankResp ResponseConstructor) (ResponseMessage, error) {
// handle protocol versions <=2.10 (API statuses was introduced in 2.11 only)

// req argument should be strengthen with type RequestMessage
statusSupported := isStatusSupported(req.(RequestMessage)) // panic is OK here for now

var (
resp ResponseMessage
err error
Expand All @@ -195,31 +172,21 @@ func (s *SignService) HandleUnaryRequest(ctx context.Context, req any, handler U
}

if err != nil {
if !statusSupported {
return nil, err
}

resp = blankResp()

setStatusV2(resp, err)
}

// sign the response
if err = signResponse(s.key, resp, statusSupported); err != nil {
return nil, err
if err = signature.SignServiceMessage(s.key, resp); err != nil {

Check warning on line 181 in pkg/services/util/sign.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/util/sign.go#L181

Added line #L181 was not covered by tests
// We can't pass this error as status code since response will be unsigned.
// Isn't expected in practice, so panic is ok here.
panic(err)

Check warning on line 184 in pkg/services/util/sign.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/util/sign.go#L184

Added line #L184 was not covered by tests
}

return resp, nil
}

func isStatusSupported(req RequestMessage) bool {
version := req.GetMetaHeader().GetVersion()

mjr := version.GetMajor()

return mjr > 2 || mjr == 2 && version.GetMinor() >= 11
}

func setStatusV2(resp ResponseMessage, err error) {
// unwrap error
for e := errors.Unwrap(err); e != nil; e = errors.Unwrap(err) {
Expand All @@ -228,22 +195,3 @@ func setStatusV2(resp ResponseMessage, err error) {

session.SetStatus(resp, apistatus.ErrorToV2(err))
}

// signs response with private key via signature.SignServiceMessage.
// The signature error affects the result depending on the protocol version:
// - if status return is supported, panics since we cannot return the failed status, because it will not be signed;
// - otherwise, returns error in order to transport it directly.
func signResponse(key *ecdsa.PrivateKey, resp any, statusSupported bool) error {
err := signature.SignServiceMessage(key, resp)
if err != nil {
err = fmt.Errorf("could not sign response: %w", err)

if statusSupported {
// We can't pass this error as status code since response will be unsigned.
// Isn't expected in practice, so panic is ok here.
panic(err)
}
}

return err
}

0 comments on commit 775f168

Please sign in to comment.