From 63560e0d1e767a8c973bfa217d3c734adea6d5f7 Mon Sep 17 00:00:00 2001 From: Ashutosh Narkar Date: Wed, 7 Oct 2020 10:27:33 -0700 Subject: [PATCH] topdown/http: handle http.send errors in-band This change updates how errors from http.send are handled. By default, an error returned by `http.send` halts the policy evaluation. This commit allows users to return errors in the response object returned by `http.send` instead of halting evaluation. Fixes: #2187 Signed-off-by: Ashutosh Narkar --- docs/content/policy-reference.md | 9 +++- topdown/http.go | 74 ++++++++++++++++++++++---- topdown/http_test.go | 90 ++++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+), 12 deletions(-) diff --git a/docs/content/policy-reference.md b/docs/content/policy-reference.md index 2ecdfbfc55..acfc2e612a 100644 --- a/docs/content/policy-reference.md +++ b/docs/content/policy-reference.md @@ -863,6 +863,7 @@ The `request` object parameter may contain the following fields: | `cache` | no | `boolean` | Cache HTTP response across OPA queries. Default: `false`. | | `force_cache` | no | `boolean` | Cache HTTP response across OPA queries and override cache directives defined by the server. Default: `false`. | | `force_cache_duration_seconds` | no | `number` | If `force_cache` is set, this field specifies the duration in seconds for the freshness of a cached response. | +| `raise_error` | no | `bool` | If `raise_error` is set, errors returned by `http.send` will halt policy evaluation. Default: `true`. | If the `Host` header is included in `headers`, its value will be used as the `Host` header of the request. The `url` parameter will continue to specify the server to connect to. @@ -879,10 +880,16 @@ The `response` object parameter will contain the following fields: | Field | Type | Description | | --- | --- | --- | | `status` | `string` | HTTP status message (e.g., `"200 OK"`). | -| `status_code` | `number` | HTTP status code (e.g., `200`). | +| `status_code` | `number` | HTTP status code (e.g., `200`). If `raise_error` is `false`, this field will be set to `0` if `http.send` encounters an error. | | `body` | `any` | Any JSON value. If the HTTP response message body was not deserialized from JSON, this field is set to `null`. | | `raw_body` | `string` | The entire raw HTTP response message body represented as a string. | | `headers` | `object` | An object containing the response headers. The values will be an array of strings, repeated headers are grouped under the same keys with all values in the array. | +| `error` | `object` | If `raise_error` is `false`, this field will represent the error encountered while running `http.send`. The `error` object contains a `message` key which holds the actual error message and a `code` key which represents if the error was caused due to a network issue or during policy evaluation. | + +By default, an error returned by `http.send` halts the policy evaluation. This behaviour can be altered such that +instead of halting evaluation, if `http.send` encounters an error, it can return a `response` object with `status_code` +set to `0` and `error` describing the actual error. This can be activated by setting the `raise_error` field +in the `request` object to `false`. If the `cache` field in the `request` object is `true`, `http.send` will return a cached response after it checks its freshness and validity. diff --git a/topdown/http.go b/topdown/http.go index 4e8fff7bbc..9bf4608395 100644 --- a/topdown/http.go +++ b/topdown/http.go @@ -53,6 +53,7 @@ var allowedKeyNames = [...]string{ "cache", "force_cache", "force_cache_duration_seconds", + "raise_error", } var ( @@ -63,46 +64,86 @@ var ( type httpSendKey string -// httpSendBuiltinCacheKey is the key in the builtin context cache that -// points to the http.send() specific cache resides at. -const httpSendBuiltinCacheKey httpSendKey = "HTTP_SEND_CACHE_KEY" +const ( + // httpSendBuiltinCacheKey is the key in the builtin context cache that + // points to the http.send() specific cache resides at. + httpSendBuiltinCacheKey httpSendKey = "HTTP_SEND_CACHE_KEY" -func builtinHTTPSend(bctx BuiltinContext, args []*ast.Term, iter func(*ast.Term) error) error { + // HTTPSendInternalErr represents a runtime evaluation error. + HTTPSendInternalErr string = "eval_http_send_internal_error" - bctx.Metrics.Timer(httpSendLatencyMetricKey).Start() + // HTTPSendNetworkErr represents a network error. + HTTPSendNetworkErr string = "eval_http_send_network_error" +) +func builtinHTTPSend(bctx BuiltinContext, args []*ast.Term, iter func(*ast.Term) error) error { req, err := validateHTTPRequestOperand(args[0], 1) if err != nil { return handleBuiltinErr(ast.HTTPSend.Name, bctx.Location, err) } + raiseError, err := getRaiseErrorValue(req) + if err != nil { + return handleBuiltinErr(ast.HTTPSend.Name, bctx.Location, err) + } + + result, err := getHTTPResponse(bctx, req) + if err != nil { + if raiseError { + return handleHTTPSendErr(bctx, err) + } + + obj := ast.NewObject() + obj.Insert(ast.StringTerm("status_code"), ast.IntNumberTerm(0)) + + errObj := ast.NewObject() + + switch err.(type) { + case *url.Error: + errObj.Insert(ast.StringTerm("code"), ast.StringTerm(HTTPSendNetworkErr)) + default: + errObj.Insert(ast.StringTerm("code"), ast.StringTerm(HTTPSendInternalErr)) + } + + errObj.Insert(ast.StringTerm("message"), ast.StringTerm(err.Error())) + obj.Insert(ast.StringTerm("error"), ast.NewTerm(errObj)) + + result = ast.NewTerm(obj) + } + return iter(result) +} + +func getHTTPResponse(bctx BuiltinContext, req ast.Object) (*ast.Term, error) { + + bctx.Metrics.Timer(httpSendLatencyMetricKey).Start() + reqExecutor, err := newHTTPRequestExecutor(bctx, req) if err != nil { - return handleHTTPSendErr(bctx, err) + return nil, err } // check if cache already has a response for this query resp, err := reqExecutor.CheckCache() if err != nil { - return handleHTTPSendErr(bctx, err) + return nil, err } if resp == nil { httpResp, err := reqExecutor.ExecuteHTTPRequest() if err != nil { - return handleHTTPSendErr(bctx, err) + return nil, err } // add result to cache resp, err = reqExecutor.InsertIntoCache(httpResp) if err != nil { - return handleHTTPSendErr(bctx, err) + return nil, err } } bctx.Metrics.Timer(httpSendLatencyMetricKey).Stop() - return iter(ast.NewTerm(resp)) + return ast.NewTerm(resp), nil } func init() { @@ -303,7 +344,7 @@ func createHTTPRequest(bctx BuiltinContext, obj ast.Object) (*http.Request, *htt if err != nil { return nil, nil, err } - case "cache", "force_cache", "force_cache_duration_seconds", "force_json_decode": // no-op + case "cache", "force_cache", "force_cache_duration_seconds", "force_json_decode", "raise_error": // no-op default: return nil, nil, fmt.Errorf("invalid parameter %q", key) } @@ -1081,3 +1122,14 @@ func newForceCacheParams(req ast.Object) (*forceCacheParams, error) { return &forceCacheParams{forceCacheDurationSeconds: int32(value)}, nil } + +func getRaiseErrorValue(req ast.Object) (bool, error) { + result := ast.Boolean(true) + var ok bool + if v := req.Get(ast.StringTerm("raise_error")); v != nil { + if result, ok = v.Value.(ast.Boolean); !ok { + return false, fmt.Errorf("invalid value for raise_error field") + } + } + return bool(result), nil +} diff --git a/topdown/http_test.go b/topdown/http_test.go index 7b1d354ff7..2a6312e53e 100644 --- a/topdown/http_test.go +++ b/topdown/http_test.go @@ -661,6 +661,96 @@ func TestHTTPRedirectEnable(t *testing.T) { runTopDownTestCase(t, data, "http.send", rules, resultObj.String()) } +func TestHTTPSendRaiseError(t *testing.T) { + + // test server + baseURL, teardown := getTestServer() + defer teardown() + + networkErrObj := make(map[string]interface{}) + networkErrObj["code"] = HTTPSendNetworkErr + networkErrObj["message"] = "Get \"foo://foo.com\": unsupported protocol scheme \"foo\"" + + networkErr, err := ast.InterfaceToValue(networkErrObj) + if err != nil { + panic(err) + } + + internalErrObj := make(map[string]interface{}) + internalErrObj["code"] = HTTPSendInternalErr + internalErrObj["message"] = fmt.Sprintf(`http.send({"method": "get", "url": "%s", "force_json_decode": true, "raise_error": false, "force_cache": true}): eval_builtin_error: http.send: 'force_cache' set but 'force_cache_duration_seconds' parameter is missing`, baseURL) + + internalErr, err := ast.InterfaceToValue(internalErrObj) + if err != nil { + panic(err) + } + + responseObj := make(map[string]interface{}) + responseObj["status_code"] = 0 + responseObj["error"] = internalErrObj + + response, err := ast.InterfaceToValue(responseObj) + if err != nil { + panic(err) + } + + tests := []struct { + note string + ruleTemplate string + body string + response interface{} + }{ + { + note: "http.send invalid url (don't raise error, check response body)", + ruleTemplate: `p = x { + r = http.send({"method": "get", "url": "%URL%.com", "force_json_decode": true, "raise_error": false}) + x = r.body + }`, + response: ``, + }, + { + note: "http.send invalid url (don't raise error, check response status code)", + ruleTemplate: `p = x { + r = http.send({"method": "get", "url": "%URL%.com", "force_json_decode": true, "raise_error": false}) + x = r.status_code + }`, + response: `0`, + }, + { + note: "http.send invalid url (don't raise error, network error)", + ruleTemplate: `p = x { + r = http.send({"method": "get", "url": "foo://foo.com", "force_json_decode": true, "raise_error": false}) + x = r.error + }`, + response: networkErr.String(), + }, + { + note: "http.send missing param (don't raise error, internal error)", + ruleTemplate: `p = x { + r = http.send({"method": "get", "url": "%URL%", "force_json_decode": true, "raise_error": false, "force_cache": true}) + x = r.error + }`, + response: internalErr.String(), + }, + { + note: "http.send missing param (don't raise error, check response)", + ruleTemplate: `p = x { + r = http.send({"method": "get", "url": "%URL%", "force_json_decode": true, "raise_error": false, "force_cache": true}) + x = r + }`, + response: response.String(), + }, + } + + data := loadSmallTestData() + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + runTopDownTestCase(t, data, tc.note, []string{strings.ReplaceAll(tc.ruleTemplate, "%URL%", baseURL)}, tc.response) + }) + } +} + func TestHTTPSendCaching(t *testing.T) { // expected result var body []interface{}