[GitHub] [apisix] spacewander commented on a change in pull request #5779: feat: supprot OPA plugin complex response

[GitBox]

spacewander commented on a change in pull request #5779:
URL: https://github.com/apache/apisix/pull/5779#discussion_r769238189



##
File path: apisix/plugins/opa.lua
##
@@ -89,13 +90,31 @@ function _M.access(conf, ctx)
 -- parse the results of the decision
 local data, err = core.json.decode(res.body)
 
-if err then
+if err or not data or not data.result then
 core.log.error("invalid response body: ", res.body, " err: ", err)

Review comment:
   We should distinguish "not data or not data.result" in the error log.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [apisix] spacewander commented on a change in pull request #5779: feat: supprot OPA plugin complex response

[GitBox]

spacewander commented on a change in pull request #5779:
URL: https://github.com/apache/apisix/pull/5779#discussion_r768264813



##
File path: apisix/plugins/opa.lua
##
@@ -94,8 +95,27 @@ function _M.access(conf, ctx)
 return 503
 end
 
-if not data.result then
-return 403
+local result = data.result
+
+if not result.allow then
+if result.headers then
+core.response.set_header(result.headers)

Review comment:
   Is there a test for multiple headers in result.headers?

##
File path: apisix/plugins/opa.lua
##
@@ -94,8 +95,27 @@ function _M.access(conf, ctx)
 return 503
 end
 
-if not data.result then
-return 403
+local result = data.result
+
+if not result.allow then
+if result.headers then
+core.response.set_header(result.headers)
+end
+
+local status_code = 403
+local reason = ""
+
+if result.status_code then
+status_code = result.status_code
+end
+
+if result.reason then
+reason = type(result.reason) == "table"

Review comment:
   Let's add test for result.reason (string and table cases).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org