Skip to content

Commit

Permalink
server: add warning when input attribute is missing
Browse files Browse the repository at this point in the history 
This change adds an api_usage_warning when input attribute is missing in the request to /v1/data

Fixes: open-policy-agent#4386
Signed-off-by: Alam <afaaqalam@gmail.com>

server: remove url from warning msg

Signed-off-by: Alam <afaaqalam@gmail.com>

server: remove warning for GET requests to v1/data; fix tests

Signed-off-by: Alam <afaaqalam@gmail.com>
  • Loading branch information
Alam committed Mar 24, 2022
1 parent 6479b80 commit 64c1262
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 14 deletions.
4 changes: 4 additions & 0 deletions server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1630,6 +1630,10 @@ func (s *Server) v1DataPost(w http.ResponseWriter, r *http.Request) {
DecisionID: decisionID,
}

if input == nil {
result.Warning = types.NewWarning(types.CodeAPIUsageWarn, types.MsgInputKeyMissing)
}

if includeMetrics || includeInstrumentation {
result.Metrics = m.All()
}
Expand Down
128 changes: 114 additions & 14 deletions server/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1310,9 +1310,26 @@ p = true { false }`
{http.MethodGet, "/data", "", 200, `{"result": {"testmod": {"p": [1,2,3,4], "q": {"a":1, "b": 2}}, "x": [1,2,3,4]}}`},
}},
{"post root", []tr{
{http.MethodPost, "/data", "", 200, `{"result": {}}`},
{http.MethodPost, "/data", "", 200, `{
"result": {},
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`},
{http.MethodPut, "/policies/test", testMod2, 200, ""},
{http.MethodPost, "/data", "", 200, `{"result": {"testmod": {"p": [1,2,3,4], "q": {"b": 2, "a": 1}}}}`},
{http.MethodPost, "/data", "", 200, `{
"result": {
"testmod": {
"p": [1,2,3,4],
"q": {"b": 2, "a": 1}
}
},
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`},
}},
{"post input", []tr{
{http.MethodPut, "/policies/test", testMod1, 200, ""},
Expand All @@ -1325,7 +1342,13 @@ p = true { false }`
}`},
}},
{"post empty object", []tr{
{http.MethodPost, "/data", `{}`, 200, `{"result": {}}`},
{http.MethodPost, "/data", `{}`, 200, `{
"result": {},
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`},
}},
{"post partial", []tr{
{http.MethodPut, "/policies/test", testMod7, 200, ""},
Expand All @@ -1348,9 +1371,20 @@ p = true { false }`
}},
{"partial invalidate data", []tr{
{http.MethodPut, "/policies/test", testMod8, 200, ""},
{http.MethodPost, "/data/testmod/p?partial", "", 200, `{}`},
{http.MethodPost, "/data/testmod/p?partial", "", 200, `{
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`},
{http.MethodPut, "/data/x", `1`, 204, ""},
{http.MethodPost, "/data/testmod/p?partial", "", 200, `{"result": true}`},
{http.MethodPost, "/data/testmod/p?partial", "", 200, `{
"result": true,
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`},
}},
{"partial ineffective fallback to normal", []tr{
{http.MethodPut, "/policies/test", testMod7, 200, ""},
Expand All @@ -1361,6 +1395,10 @@ p = true { false }`
"q": [],
"r": []
}
},
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`},
{http.MethodPost, "/data", "", 200, `{
Expand All @@ -1370,6 +1408,10 @@ p = true { false }`
"q": [],
"r": []
}
},
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`},
}},
Expand Down Expand Up @@ -1417,10 +1459,32 @@ p = true { false }`
{http.MethodGet, "/data", "", 200, `{"result": {"a/b": {"c/d": 1}}}`},
{http.MethodGet, "/data/a%2Fb/c%2Fd", "", 200, `{"result": 1}`},
{http.MethodGet, "/data/a/b", "", 200, `{}`},
{http.MethodPost, "/data/a%2Fb/c%2Fd", "", 200, `{"result": 1}`},
{http.MethodPost, "/data/a/b", "", 200, `{}`},
{http.MethodPost, "/data/a%2Fb/c%2Fd", "", 200, `{
"result": 1,
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`},
{http.MethodPost, "/data/a/b", "", 200, `{
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`},
{http.MethodPatch, "/data/a%2Fb", `[{"op": "add", "path": "/e%2Ff", "value": 2}]`, 204, ""},
{http.MethodPost, "/data", "", 200, `{"result": {"a/b": {"c/d": 1, "e/f": 2}}}`},
{http.MethodPost, "/data", "", 200, `{
"result": {
"a/b": {
"c/d": 1,
"e/f": 2
}
},
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`},
}},
{"strict-builtin-errors", []tr{
{http.MethodPut, "/policies/test", `
Expand All @@ -1446,7 +1510,13 @@ p = true { false }`
}
]
}`},
{http.MethodPost, "/data/test/p", "", 200, `{"result": false}`},
{http.MethodPost, "/data/test/p", "", 200, `{
"result": false,
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`},
{http.MethodPost, "/data/test/p?strict-builtin-errors", "", 500, `{
"code": "internal_error",
"message": "error(s) occurred while evaluating query",
Expand All @@ -1463,6 +1533,16 @@ p = true { false }`
]
}`},
}},
{"post api usage warning", []tr{
{http.MethodPost, "/data", "", 200, `{
"result": {},
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`},
{http.MethodPost, "/data", `{"input": {}}`, 200, `{"result": {}}`},
}},
}

for _, tc := range tests {
Expand Down Expand Up @@ -2823,15 +2903,28 @@ func TestDecisionIDs(t *testing.T) {
t.Fatal(err)
}

if err := f.v1("POST", "/data/undefined", "", 200, `{"decision_id": "2"}`); err != nil {
if err := f.v1("POST", "/data/undefined", "", 200, `{
"decision_id": "2",
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`); err != nil {
t.Fatal(err)
}

if err := f.v1("GET", "/data", "", 200, `{"decision_id": "3", "result": {}}`); err != nil {
t.Fatal(err)
}

if err := f.v1("POST", "/data", "", 200, `{"decision_id": "4", "result": {}}`); err != nil {
if err := f.v1("POST", "/data", "", 200, `{
"decision_id": "4",
"result": {},
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
}
}`); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -2876,9 +2969,16 @@ func TestDecisionLogging(t *testing.T) {
response: "{}",
},
{
method: "POST",
path: "/data",
response: `{"result": {}, "decision_id": "1"}`,
method: "POST",
path: "/data",
response: `{
"result": {},
"warning": {
"code": "api_usage_warning",
"message": "'input' key missing from the request"
},
"decision_id": "1"
}`,
},
{
method: "GET",
Expand Down
18 changes: 18 additions & 0 deletions server/types/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,24 @@ type DataResponseV1 struct {
Explanation TraceV1 `json:"explanation,omitempty"`
Metrics MetricsV1 `json:"metrics,omitempty"`
Result *interface{} `json:"result,omitempty"`
Warning *Warning `json:"warning,omitempty"`
}

// Warning models DataResponse warnings
type Warning struct {
Code string `json:"code,omitempty"`
Message string `json:"message,omitempty"`
}

// Warning Codes
const CodeAPIUsageWarn = "api_usage_warning"

// Warning Messages
const MsgInputKeyMissing = "'input' key missing from the request"

// NewWarning returns a new Warning object
func NewWarning(code, message string) *Warning {
return &Warning{Code: code, Message: message}
}

// MetricsV1 models a collection of performance metrics.
Expand Down

0 comments on commit 64c1262

Please sign in to comment.