Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backend error handler #490

Merged
merged 17 commits into from
Apr 28, 2022
1 change: 1 addition & 0 deletions config/request/context_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const (
AccessControls
BackendName
BackendParams
BackendTokenRequest
BetaGrantedPermissions
BetaRequiredPermission
BufferOptions
Expand Down
50 changes: 38 additions & 12 deletions config/runtime/error_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,33 @@ import (
"github.com/avenga/couper/handler"
)

var wildcardMap = map[string][]string{
"api": {"beta_insufficient_permissions"},
"endpoint": {"beta_insufficient_permissions", "sequence", "unexpected_status"},
type wildcardValues map[string][]string

var wildcardMap = map[string]wildcardValues{
johakoch marked this conversation as resolved.
Show resolved Hide resolved
"api": {
"backend": {
"backend_openapi_validation",
"backend_timeout",
},
"*": {
"backend_openapi_validation",
"backend_timeout",
"beta_insufficient_permissions",
},
},
"endpoint": {
"backend": {
"backend_openapi_validation",
"backend_timeout",
},
"*": {
"backend_openapi_validation",
"backend_timeout",
"beta_insufficient_permissions",
"sequence",
"unexpected_status",
},
},
}

func newErrorHandler(ctx *hcl.EvalContext, opts *protectedOptions, log *logrus.Entry,
Expand All @@ -33,19 +57,21 @@ func newErrorHandler(ctx *hcl.EvalContext, opts *protectedOptions, log *logrus.E
}
}

// this works if wildcard is the only "super kind"
if mappedKinds, mkExists := wildcardMap[ref]; mkExists {
// expand wildcard:
// * set wildcard error handler for mapped kinds, no error handler for this kind is already set
if mappedValues, mkExists := wildcardMap[ref]; mkExists {
// expand wildcards:
// * set wildcard error handler for mapped kinds, if no error handler for this kind is already set
// * remove wildcard error handler for wildcard
if wcHandler, wcExists := handlersPerKind["*"]; wcExists {
for _, mk := range mappedKinds {
if _, exists := handlersPerKind[mk]; !exists {
handlersPerKind[mk] = wcHandler
for key, values := range mappedValues {
if wcHandler, wcExists := handlersPerKind[key]; wcExists {
for _, mk := range values {
if _, exists := handlersPerKind[mk]; !exists {
handlersPerKind[mk] = wcHandler
}
}

delete(handlersPerKind, key)
}
}
delete(handlersPerKind, "*")
}

for k, h := range handlersPerKind {
Expand Down
6 changes: 6 additions & 0 deletions docs/ERRORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,17 @@ All errors have a specific type. You can find it in the log field `error_type`.

| Type (and super types) | Description | Default handling |
|:------------------------------------------------|:--------------------------------------------------------------------------------------------------------|:----------------------------------------------------------------------------|
| `backend` | All catchable `backend` related errors | Send error template with status `502`. |
| `backend_openapi_validation` (`backend`) | Client request or backend response is invalid | Send error template with status code `400` for invalid client request or `502` for invalid backend response. |
| `backend_timeout` (`backend`) | A backend request timed out | Send error template with status `504`. |
| `beta_insufficient_permissions` | The permission required for the requested operation is not in the permissions granted to the requester. | Send error template with status `403`. |

### Endpoint error types

| Type (and super types) | Description | Default handling |
|:------------------------------------------------|:-------------------------------------------------------------------------------------------------|:----------------------------------------------------------------------------|
| `backend` | All catchable `backend` related errors | Send error template with status `502`. |
| `backend_openapi_validation` (`backend`) | Client request or backend response is invalid | Send error template with status code `400` for invalid client request or `502` for invalid backend response. |
| `backend_timeout` (`backend`) | A backend request timed out | Send error template with status `504`. |
| `sequence` | A `request` or `proxy` block request has been failed while depending on another one | Send error template with status `502`. |
| `unexpected_status` | A `request` or `proxy` block response status code does not match the to `expected_status` list | Send error template with status `502`. |
26 changes: 12 additions & 14 deletions errors/couper.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,18 @@ import (
const Wildcard = "*"

var (
AccessControl = &Error{synopsis: "access control error", kinds: []string{"access_control"}, httpStatus: http.StatusForbidden}
Backend = &Error{synopsis: "backend error", httpStatus: http.StatusBadGateway}
BackendTimeout = &Error{synopsis: "backend timeout error", httpStatus: http.StatusGatewayTimeout}
BackendValidation = &Error{synopsis: "backend validation error", kinds: []string{"backend_validation"}, httpStatus: http.StatusBadRequest}
ClientRequest = &Error{synopsis: "client request error", httpStatus: http.StatusBadRequest}
Endpoint = &Error{synopsis: "endpoint error", kinds: []string{"endpoint"}, httpStatus: http.StatusBadGateway}
Evaluation = &Error{synopsis: "expression evaluation error", kinds: []string{"evaluation"}, httpStatus: http.StatusInternalServerError}
Configuration = &Error{synopsis: "configuration error", kinds: []string{"configuration"}, httpStatus: http.StatusInternalServerError}
MethodNotAllowed = &Error{synopsis: "method not allowed error", httpStatus: http.StatusMethodNotAllowed}
Proxy = &Error{synopsis: "proxy error", httpStatus: http.StatusBadGateway}
Request = &Error{synopsis: "request error", httpStatus: http.StatusBadGateway}
RouteNotFound = &Error{synopsis: "route not found error", httpStatus: http.StatusNotFound}
Server = &Error{synopsis: "internal server error", httpStatus: http.StatusInternalServerError}
ServerShutdown = &Error{synopsis: "server shutdown error", httpStatus: http.StatusInternalServerError}
AccessControl = &Error{synopsis: "access control error", kinds: []string{"access_control"}, httpStatus: http.StatusForbidden}
Backend = &Error{synopsis: "backend error", kinds: []string{"backend"}, httpStatus: http.StatusBadGateway}
ClientRequest = &Error{synopsis: "client request error", httpStatus: http.StatusBadRequest}
Endpoint = &Error{synopsis: "endpoint error", kinds: []string{"endpoint"}, httpStatus: http.StatusBadGateway}
Evaluation = &Error{synopsis: "expression evaluation error", kinds: []string{"evaluation"}, httpStatus: http.StatusInternalServerError}
Configuration = &Error{synopsis: "configuration error", kinds: []string{"configuration"}, httpStatus: http.StatusInternalServerError}
MethodNotAllowed = &Error{synopsis: "method not allowed error", httpStatus: http.StatusMethodNotAllowed}
Proxy = &Error{synopsis: "proxy error", httpStatus: http.StatusBadGateway}
Request = &Error{synopsis: "request error", httpStatus: http.StatusBadGateway}
RouteNotFound = &Error{synopsis: "route not found error", httpStatus: http.StatusNotFound}
Server = &Error{synopsis: "internal server error", httpStatus: http.StatusInternalServerError}
ServerShutdown = &Error{synopsis: "server shutdown error", httpStatus: http.StatusInternalServerError}
)

func TypeToSnake(t interface{}) string {
Expand Down
4 changes: 3 additions & 1 deletion errors/type_defintions.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ var Definitions = []*Error{

AccessControl.Kind("beta_insufficient_permissions"),

BackendValidation,
Backend,
Backend.Kind("backend_openapi_validation").Status(http.StatusBadRequest),
Backend.Kind("backend_timeout").Status(http.StatusGatewayTimeout),

Endpoint.Kind("sequence"),
Endpoint.Kind("unexpected_status"),
Expand Down
10 changes: 7 additions & 3 deletions errors/types_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 5 additions & 8 deletions handler/transport/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func (b *Backend) RoundTrip(req *http.Request) (*http.Response, error) {
func (b *Backend) openAPIValidate(req *http.Request, tc *Config, deadlineErr <-chan error) (*http.Response, error) {
requestValidationInput, err := b.openAPIValidator.ValidateRequest(req)
if err != nil {
return nil, errors.BackendValidation.Label(b.name).Kind("backend_request_validation").With(err)
return nil, errors.BackendOpenapiValidation.Label(b.name).With(err)
}

beresp, err := b.innerRoundTrip(req, tc, deadlineErr)
Expand All @@ -225,8 +225,7 @@ func (b *Backend) openAPIValidate(req *http.Request, tc *Config, deadlineErr <-c
}

if err = b.openAPIValidator.ValidateResponse(beresp, requestValidationInput); err != nil {
return nil, errors.BackendValidation.Label(b.name).Kind("backend_response_validation").
With(err).Status(http.StatusBadGateway)
return nil, errors.BackendOpenapiValidation.Label(b.name).With(err).Status(http.StatusBadGateway)
}

return beresp, nil
Expand Down Expand Up @@ -283,19 +282,17 @@ func (b *Backend) innerRoundTrip(req *http.Request, tc *Config, deadlineErr <-ch
return beresp, nil
}

const backendTokenRequest = "backendTokenRequest"

func (b *Backend) withTokenRequest(req *http.Request) error {
if b.tokenRequest == nil {
return nil
}

trValue, _ := req.Context().Value(backendTokenRequest).(string)
trValue, _ := req.Context().Value(request.BackendTokenRequest).(string)
if trValue != "" { // prevent loop
return nil
}

ctx := context.WithValue(req.Context(), backendTokenRequest, "tr")
ctx := context.WithValue(req.Context(), request.BackendTokenRequest, "tr")
// Reset for upstream transport; prevent mixing values.
// tokenRequest will have their own backend configuration.
ctx = context.WithValue(ctx, request.BackendParams, nil)
Expand All @@ -310,7 +307,7 @@ func (b *Backend) withRetryTokenRequest(req *http.Request, res *http.Response) (
return false, nil
}

trValue, _ := req.Context().Value(backendTokenRequest).(string)
trValue, _ := req.Context().Value(request.BackendTokenRequest).(string)
if trValue != "" { // prevent loop
return false, nil
}
Expand Down
4 changes: 2 additions & 2 deletions handler/transport/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func TestBackend_RoundTrip_Validation(t *testing.T) {
&config.OpenAPI{File: "testdata/upstream.yaml"},
http.MethodPost,
"/get",
"backend validation error",
"backend error",
"'POST /get': method not allowed",
},
{
Expand All @@ -219,7 +219,7 @@ func TestBackend_RoundTrip_Validation(t *testing.T) {
&config.OpenAPI{File: "testdata/upstream.yaml"},
http.MethodGet,
"/get?404",
"backend validation error",
"backend error",
"status is not supported",
},
{
Expand Down
4 changes: 2 additions & 2 deletions handler/validation/openapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ func TestOpenAPIValidator_ValidateRequest(t *testing.T) {
wantBody bool
wantErrLog string
}{
{"GET without required query", "/a?b", nil, false, `backend validation error: parameter "b" in query has an error: value is required but missing`},
{"GET without required query", "/a?b", nil, false, `backend error: parameter "b" in query has an error: value is required but missing`},
{"GET with required query", "/a?b=value", nil, false, ""},
{"GET with required path", "/a/value", nil, false, ""},
{"GET with required path missing", "/a//", nil, false, `backend validation error: parameter "b" in query has an error: value is required but missing`},
{"GET with required path missing", "/a//", nil, false, `backend error: parameter "b" in query has an error: value is required but missing`},
{"GET with optional query", "/b", nil, false, ""},
{"GET with optional path param", "/b/a", nil, false, ""},
{"GET with required json body", "/json", strings.NewReader(`["hans", "wurst"]`), true, ""},
Expand Down
2 changes: 1 addition & 1 deletion server/http_endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,7 @@ func TestEndpointSequenceBackendTimeout(t *testing.T) {
}

path := entry.Data["request"].(logging.Fields)["path"]
if entry.Message == "backend timeout error: anonymous_3_23: deadline exceeded" {
if entry.Message == "backend error: anonymous_3_23: deadline exceeded" {
ctxDeadlineSeen = true
if path != "/" {
t.Errorf("expected '/' to fail")
Expand Down
48 changes: 48 additions & 0 deletions server/http_error_handler_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package server_test

import (
"bytes"
"io"
"net/http"
"strings"
"testing"
Expand Down Expand Up @@ -123,6 +125,52 @@ func TestAccessControl_ErrorHandler_Configuration_Error(t *testing.T) {
}
}

func TestErrorHandler_Backend(t *testing.T) {
client := test.NewHTTPClient()

shutdown, _ := newCouper("testdata/integration/error_handler/05_couper.hcl", test.New(t))
defer shutdown()

type testcase struct {
path string
expBody string
expStatus int
}

for _, tc := range []testcase{
{"/api-backend", `{"api":"backend"}`, 405},
{"/api-backend-timeout", `{"api":"backend-timeout"}`, 405},
{"/api-backend-validation", `{"api":"backend-validation"}`, 405},
{"/api-anything", `{"api":"backend-backend-validation"}`, 405},
{"/backend", `{"endpoint":"backend"}`, 405},
{"/backend-timeout", `{"endpoint":"backend-timeout"}`, 405},
{"/backend-validation", `{"endpoint":"backend-validation"}`, 405},
{"/anything", `{"endpoint":"backend-backend-validation"}`, 405},
} {
johakoch marked this conversation as resolved.
Show resolved Hide resolved
t.Run(tc.path, func(st *testing.T) {
helper := test.New(st)

req, err := http.NewRequest(http.MethodGet, "http://anyserver:8080"+tc.path, nil)
helper.Must(err)

res, err := client.Do(req)
helper.Must(err)

if res.StatusCode != tc.expStatus {
st.Fatalf("want: %d, got: %d", tc.expStatus, res.StatusCode)
}

resBytes, err := io.ReadAll(res.Body)
defer res.Body.Close()
helper.Must(err)

if !bytes.Contains(resBytes, []byte(tc.expBody)) {
st.Fatalf("want: %s, got: %s", tc.expBody, resBytes)
}
})
}
}

func TestAccessControl_ErrorHandler_Permissions(t *testing.T) {
client := test.NewHTTPClient()

Expand Down
13 changes: 13 additions & 0 deletions server/testdata/integration/error_handler/01_schema.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
openapi: '3'
info:
title: 'Test'
version: 'v1.2.3'
paths:
/api-backend-validation:
get:
requestBody:
content:
application/json:
schema:
type: object
required: true
10 changes: 10 additions & 0 deletions server/testdata/integration/error_handler/02_schema.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
openapi: '3'
info:
title: 'Test'
version: 'v1.2.3'
paths:
/anything:
get:
responses:
405:
description: OK
Loading