Skip to content

Commit

Permalink
topdown/http: handle http.send errors in-band
Browse files Browse the repository at this point in the history
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 <anarkar4387@gmail.com>
  • Loading branch information
ashutosh-narkar committed Oct 9, 2020
1 parent 4851c4b commit 63560e0
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 12 deletions.
9 changes: 8 additions & 1 deletion docs/content/policy-reference.md
Expand Up @@ -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.

Expand All @@ -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.

Expand Down
74 changes: 63 additions & 11 deletions topdown/http.go
Expand Up @@ -53,6 +53,7 @@ var allowedKeyNames = [...]string{
"cache",
"force_cache",
"force_cache_duration_seconds",
"raise_error",
}

var (
Expand All @@ -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() {
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
90 changes: 90 additions & 0 deletions topdown/http_test.go
Expand Up @@ -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{}
Expand Down

0 comments on commit 63560e0

Please sign in to comment.