diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index e7ed3251..2da868f2 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -24,6 +24,9 @@ jobs: run: | go get -v -t -d ./... + - name: Build RDMA session server archive + run: make rdma/librcserver.a + - name: Test run: go test -coverprofile profile.txt -race -v -timeout 30s -tags=github ./... diff --git a/rdma/rcroutes/errors.go b/rdma/rcroutes/errors.go index ee6221a7..5f3d0627 100644 --- a/rdma/rcroutes/errors.go +++ b/rdma/rcroutes/errors.go @@ -18,26 +18,41 @@ import ( "github.com/gofiber/fiber/v3" + "github.com/versity/versitygw/debuglogger" "github.com/versity/versitygw/s3api/utils" "github.com/versity/versitygw/s3err" ) -// writeRouteError renders err as the terminal S3-style XML response +// ErrRouteNotImplemented is the platform-stub answer of the +// control routes on builds without the RC data plane. +type ErrRouteNotImplemented struct{} + +func (ErrRouteNotImplemented) Error() string { + return "RDMA not supported on this platform" +} + +// isRouteNotImplemented reports whether err is the platform-stub +// answer of the control routes. The marker type is shared, so +// every build classifies it the same way. +func isRouteNotImplemented(err error) bool { + var ni ErrRouteNotImplemented + return errors.As(err, &ni) +} + +// WriteRouteError renders err as the terminal S3-style XML response // of an RC control route. The production S3 error handler converts // ordinary Fiber errors into a generic 500 response, so the route // must send its final status and body itself. Errors that already // carry S3 semantics (authentication, authorization, object // backend) keep their status and code; anything else is mapped to -// a protocol error without exposing internal detail. -func writeRouteError(ctx fiber.Ctx, err error) error { +// a protocol error without exposing internal detail. The gateway +// auth adapter uses it for the same reason. +func WriteRouteError(ctx fiber.Ctx, err error) error { requestID, hostID := utils.EnsureRequestIDs(ctx) - var apiErr s3err.APIError - switch e := classifyRouteError(err).(type) { - case s3err.S3Error: - apiErr = e.BaseError() - case s3err.APIError: - apiErr = e + apiErr := routeError(err) + if apiErr.HTTPStatusCode == fiber.StatusInternalServerError { + logInternalRouteError(ctx, err) } if isRouteNotImplemented(err) { apiErr = s3err.APIError{ @@ -52,13 +67,14 @@ func writeRouteError(ctx fiber.Ctx, err error) error { Send(apiErr.XMLBody(requestID, hostID)) } -// classifyRouteError resolves err to a value implementing -// s3err.S3Error. S3-aware errors pass through unchanged; RC -// transport errors are mapped to the closest protocol error. -func classifyRouteError(err error) s3err.S3Error { +// routeError resolves err to its S3-style response. Errors that +// already carry S3 semantics (authentication, authorization, +// object backend) keep their status and code; RC transport errors +// map to the closest protocol error. +func routeError(err error) s3err.APIError { var s3Err s3err.S3Error if errors.As(err, &s3Err) { - return s3Err + return s3Err.BaseError() } code, status, description := routeErrorDetails(err) @@ -69,6 +85,14 @@ func classifyRouteError(err error) s3err.S3Error { } } +// logInternalRouteError keeps a server-side trace of unexpected +// failures. The wire response stays generic; without this the +// terminal serializer would hide the production diagnostics the +// global error handler used to log. +func logInternalRouteError(ctx fiber.Ctx, err error) { + debuglogger.InternalError(err) +} + // routeErrorDetails maps a non-S3 route error to its protocol // error code, HTTP status, and description. func routeErrorDetails(err error) (code string, status int, description string) { diff --git a/rdma/rcroutes/errors_linux.go b/rdma/rcroutes/errors_linux.go index bdd7b3b6..25b9f4a1 100644 --- a/rdma/rcroutes/errors_linux.go +++ b/rdma/rcroutes/errors_linux.go @@ -83,7 +83,3 @@ func isRouteUnavailable(err error) bool { var unavail errRouteUnavailable return errors.As(err, &unavail) } - -// isRouteNotImplemented reports whether the platform stub answered -// the request; always false on Linux builds. -func isRouteNotImplemented(err error) bool { return false } diff --git a/rdma/rcroutes/errors_mapping_linux_test.go b/rdma/rcroutes/errors_mapping_linux_test.go new file mode 100644 index 00000000..74adc866 --- /dev/null +++ b/rdma/rcroutes/errors_mapping_linux_test.go @@ -0,0 +1,130 @@ +// Copyright 2026 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an "AS +// IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language +// governing permissions and limitations under the License. + +//go:build linux && amd64 && cgo + +package rcroutes + +import ( + "errors" + "fmt" + "net/http" + "testing" + + "github.com/versity/versitygw/rdma/rcserver" +) + +// This file verifies the mapping from session-server failures to +// protocol error responses. It runs where the Linux session +// server links; the s3api package exercises the shared +// serializer through the production S3 server on every platform. + +func TestRouteErrorProtocolMapping(t *testing.T) { + tests := []struct { + name string + err error + code string + want int + }{ + { + name: "invalid header is 400", + err: invalidHeader(hdrProtocol, "bogus"), + code: "InvalidRdmaRequest", + want: http.StatusBadRequest, + }, + { + name: "unknown session is 404", + err: rcserver.ErrNoSession, + code: "NoSuchRdmaSession", + want: http.StatusNotFound, + }, + { + name: "owner mismatch answers as unknown session", + err: fmt.Errorf("session owner mismatch: %w", rcserver.ErrNoSession), + code: "NoSuchRdmaSession", + want: http.StatusNotFound, + }, + { + name: "stale session is 409", + err: rcserver.ErrStale, + code: "RdmaSessionConflict", + want: http.StatusConflict, + }, + { + name: "duplicate borrow is 409", + err: fmt.Errorf("peer busy: %w", rcserver.ErrDouble), + code: "RdmaSessionConflict", + want: http.StatusConflict, + }, + { + name: "wrong session state is 409", + err: rcserver.ErrState, + code: "RdmaSessionConflict", + want: http.StatusConflict, + }, + { + name: "resource limit is 429", + err: rcserver.ErrLimit, + code: "RdmaResourceLimit", + want: http.StatusTooManyRequests, + }, + { + name: "wire failure is 502", + err: rcserver.ErrWire, + code: "RdmaTransferFailed", + want: http.StatusBadGateway, + }, + { + name: "short transfer is 400", + err: rcserver.ErrShort, + code: "InvalidRdmaRequest", + want: http.StatusBadRequest, + }, + { + name: "value too long is 400", + err: rcserver.ErrTrunc, + code: "InvalidRdmaRequest", + want: http.StatusBadRequest, + }, + { + name: "invalid argument is 400", + err: rcserver.ErrArg, + code: "InvalidRdmaRequest", + want: http.StatusBadRequest, + }, + { + name: "admission refusal is 503", + err: errNotAdmitted(), + code: "RdmaServiceUnavailable", + want: http.StatusServiceUnavailable, + }, + { + name: "unexpected error is 500", + err: errors.New("boom"), + code: "InternalError", + want: http.StatusInternalServerError, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := routeError(tt.err) + if got.HTTPStatusCode != tt.want { + t.Fatalf("status = %d, want %d", got.HTTPStatusCode, tt.want) + } + if got.Code != tt.code { + t.Fatalf("code = %q, want %q", got.Code, tt.code) + } + }) + } +} diff --git a/rdma/rcroutes/errors_stub.go b/rdma/rcroutes/errors_stub.go index 4d8af5bb..97e1c178 100644 --- a/rdma/rcroutes/errors_stub.go +++ b/rdma/rcroutes/errors_stub.go @@ -15,23 +15,6 @@ package rcroutes -import "errors" - -// errRouteNotImplemented marks the platform-stub answer of the -// control routes. -type errRouteNotImplemented struct{} - -func (errRouteNotImplemented) Error() string { - return "RDMA not supported on this platform" -} - -// isRouteNotImplemented reports whether the platform stub answered -// the request. -func isRouteNotImplemented(err error) bool { - var ni errRouteNotImplemented - return errors.As(err, &ni) -} - // isRouteBadRequest reports whether err is a malformed-request // class error. The stub never classifies request errors because it // answers before validation. diff --git a/rdma/rcroutes/routes_linux.go b/rdma/rcroutes/routes_linux.go index af1d7a18..e0a6c818 100644 --- a/rdma/rcroutes/routes_linux.go +++ b/rdma/rcroutes/routes_linux.go @@ -112,7 +112,7 @@ func (h *Handler) Prepare(ctx fiber.Ctx) error { // production S3 error handler turns ordinary Fiber // errors into a generic 500 response. if err := h.prepareCore(ctx); err != nil { - return writeRouteError(ctx, err) + return WriteRouteError(ctx, err) } return nil } @@ -269,7 +269,7 @@ func (h *Handler) Ready(ctx fiber.Ctx) error { // production S3 error handler turns ordinary Fiber // errors into a generic 500 response. if err := h.readyCore(ctx); err != nil { - return writeRouteError(ctx, err) + return WriteRouteError(ctx, err) } return nil } @@ -462,7 +462,7 @@ func (h *Handler) Cancel(ctx fiber.Ctx) error { // production S3 error handler turns ordinary Fiber // errors into a generic 500 response. if err := h.cancelCore(ctx); err != nil { - return writeRouteError(ctx, err) + return WriteRouteError(ctx, err) } return nil } diff --git a/rdma/rcroutes/routes_stub.go b/rdma/rcroutes/routes_stub.go index ad8dc60b..387f4a5e 100644 --- a/rdma/rcroutes/routes_stub.go +++ b/rdma/rcroutes/routes_stub.go @@ -37,7 +37,7 @@ func New(svc any, be backend.Backend, iam auth.IAMService, // notImplemented answers 501 through the shared terminal error // serializer so the response shape matches the Linux routes. func notImplemented(ctx fiber.Ctx) error { - return writeRouteError(ctx, errRouteNotImplemented{}) + return WriteRouteError(ctx, ErrRouteNotImplemented{}) } // Prepare is a stub handler that answers 501 Not Implemented. diff --git a/s3api/rc_route_error_test.go b/s3api/rc_route_error_test.go new file mode 100644 index 00000000..680677b5 --- /dev/null +++ b/s3api/rc_route_error_test.go @@ -0,0 +1,158 @@ +// Copyright 2026 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an "AS +// IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language +// governing permissions and limitations under the License. + +package s3api + +import ( + "encoding/xml" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gofiber/fiber/v3" + + "github.com/versity/versitygw/rdma/rcroutes" + "github.com/versity/versitygw/s3err" +) + +// This file pins the wire behavior of RDMA control-route error +// responses at the production server boundary: the real error +// handler, the real request-ID middleware, and the terminal +// serializer the RDMA routes use. The RC route handlers live in +// rdma/rcroutes; these tests exercise the same response path a +// client observes. + +type rcErrorResponse struct { + XMLName xml.Name `xml:"Error"` + Code string `xml:"Code"` + Message string `xml:"Message"` + RequestID string `xml:"RequestId"` + HostID string `xml:"HostId"` +} + +func TestRCRouteErrorPreservesS3Error(t *testing.T) { + server, err := newTestS3ApiServer( + WithRoute(http.MethodPost, "/.hipobj-rc/op", func(ctx fiber.Ctx) error { + return rcroutes.WriteRouteError(ctx, + s3err.GetAPIError(s3err.ErrAccessDenied)) + }), + ) + if err != nil { + t.Fatalf("New() error = %v", err) + } + + resp, err := server.app.Test(httptest.NewRequest(http.MethodPost, "/.hipobj-rc/op", nil)) + if err != nil { + t.Fatalf("app.Test() error = %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusForbidden) + } + var er rcErrorResponse + if err := xml.NewDecoder(resp.Body).Decode(&er); err != nil { + t.Fatalf("decode XML body: %v", err) + } + if er.Code != "AccessDenied" { + t.Fatalf("code = %q, want AccessDenied", er.Code) + } + if er.RequestID == "" || er.HostID == "" { + t.Fatal("response missing RequestId or HostId") + } + if ct := resp.Header.Get("Content-Type"); ct != fiber.MIMEApplicationXML { + t.Fatalf("content-type = %q, want %q", ct, fiber.MIMEApplicationXML) + } +} + +func TestRCRouteErrorRawFiberIs500(t *testing.T) { + // Control case: an ordinary fiber error collapses into the + // generic 500 of the production error handler. This is the + // behavior WriteRouteError exists to avoid. + server, err := newTestS3ApiServer( + WithRoute(http.MethodPost, "/.hipobj-rc/op", func(ctx fiber.Ctx) error { + return fiber.NewError(fiber.StatusTeapot, "raw") + }), + ) + if err != nil { + t.Fatalf("New() error = %v", err) + } + + resp, err := server.app.Test(httptest.NewRequest(http.MethodPost, "/.hipobj-rc/op", nil)) + if err != nil { + t.Fatalf("app.Test() error = %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusInternalServerError { + t.Fatalf("status = %d, want %d (production handler collapses fiber errors)", + resp.StatusCode, http.StatusInternalServerError) + } +} + +func TestRCRouteErrorUnexpectedIsGeneric500(t *testing.T) { + server, err := newTestS3ApiServer( + WithRoute(http.MethodPost, "/.hipobj-rc/op", func(ctx fiber.Ctx) error { + return rcroutes.WriteRouteError(ctx, errors.New("boom")) + }), + ) + if err != nil { + t.Fatalf("New() error = %v", err) + } + + resp, err := server.app.Test(httptest.NewRequest(http.MethodPost, "/.hipobj-rc/op", nil)) + if err != nil { + t.Fatalf("app.Test() error = %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusInternalServerError { + t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusInternalServerError) + } + var er rcErrorResponse + if err := xml.NewDecoder(resp.Body).Decode(&er); err != nil { + t.Fatalf("decode XML body: %v", err) + } + if er.Code != "InternalError" { + t.Fatalf("code = %q, want InternalError", er.Code) + } +} + +func TestRCRouteErrorStubNotImplemented(t *testing.T) { + server, err := newTestS3ApiServer( + WithRoute(http.MethodPost, "/.hipobj-rc/op", func(ctx fiber.Ctx) error { + return rcroutes.WriteRouteError(ctx, rcroutes.ErrRouteNotImplemented{}) + }), + ) + if err != nil { + t.Fatalf("New() error = %v", err) + } + + resp, err := server.app.Test(httptest.NewRequest(http.MethodPost, "/.hipobj-rc/op", nil)) + if err != nil { + t.Fatalf("app.Test() error = %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusNotImplemented { + t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusNotImplemented) + } + var er rcErrorResponse + if err := xml.NewDecoder(resp.Body).Decode(&er); err != nil { + t.Fatalf("decode XML body: %v", err) + } + if er.Code != "NotImplemented" { + t.Fatalf("code = %q, want NotImplemented", er.Code) + } +}