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.

Sign up for GitHub

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

Jump to bottom

feat: allow sending headers upstream returned by OPA server #9710

Merged
merged 11 commits into from
Jul 19, 2023
Merged

feat: allow sending headers upstream returned by OPA server #9710

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8f27fc5
feat: allow sending headers upstream returned by OPA server
Revolyssup Jun 21, 2023
e2e72cb
fix lint fail
Revolyssup Jun 22, 2023
3fc50ae
add test cases
Revolyssup Jun 26, 2023
47397d9
fix lint
Revolyssup Jun 26, 2023
d91053c
pass selected headers to upstream
Revolyssup Jul 17, 2023
ba5e71b
change type of items in array
Revolyssup Jul 18, 2023
418a29e
remove redundant line
Revolyssup Jul 18, 2023
8aaf9c5
add code suggestions
Revolyssup Jul 18, 2023
46aff0d
refactor with code suggestions
Revolyssup Jul 18, 2023
a784d6a
fix lint
Revolyssup Jul 18, 2023
662d321
localize ipairs
Revolyssup Jul 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions apisix/plugins/opa.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ local core = require("apisix.core")
local http = require("resty.http")
local helper = require("apisix.plugins.opa.helper")
local type = type
local pairs = pairs
monkeyDluffy6017 marked this conversation as resolved.
Show resolved Hide resolved

local schema = {
type = "object",
Expand All @@ -37,6 +38,14 @@ local schema = {
description = "timeout in milliseconds",
},
keepalive = {type = "boolean", default = true},
send_headers_upstream = {
type = "array",
minItems = 1,
items = {
type = "string"
},
description = "list of headers to pass to upstream in request"
},
keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
keepalive_pool = {type = "integer", minimum = 1, default = 5},
with_route = {type = "boolean", default = false},
Expand Down Expand Up @@ -125,6 +134,18 @@ function _M.access(conf, ctx)
end

return status_code, reason
else if result.headers and conf.send_headers_upstream then
local headersToSend = {}
for key in pairs(conf.send_headers_upstream) do
headersToSend[key] = true
end
for key,value in pairs(result.headers) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for key,value in pairs(result.headers) do
for key, value in pairs(result.headers) do

if headersToSend[key] then
core.request.set_header(ctx,key,value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
core.request.set_header(ctx,key,value)
core.request.set_header(ctx, key, value)

Copy link
Contributor

@monkeyDluffy6017 monkeyDluffy6017 Jul 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could refactor this block like this:

        for _, name in ipairs(conf.send_headers_upstream) do
            if result.headers[name] then
                core.request.set_header(ctx, name, value)
            end
        end

Copy link
Contributor Author

@Revolyssup Revolyssup Jul 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes more sense. Done

end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blank is needed

end
end
end
end

Expand Down
10 changes: 10 additions & 0 deletions ci/pod/opa/example.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ allow {
request.query["user"]
}

allow {
request.method == "GET"
startswith(request.path, "/echo")
}

reason = users[request.query["user"]].reason {
not allow
request.query["user"]
Expand All @@ -39,6 +44,11 @@ headers = users[request.query["user"]].headers {
request.query["user"]
}

headers = {"user": request.query["user"]} {
allow
request.query["user"]
}

status_code = users[request.query["user"]].status_code {
not allow
request.query["user"]
Expand Down
44 changes: 44 additions & 0 deletions t/plugin/opa.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,47 @@ test-header: only-for-test
--- error_code: 403
--- response
{"code":40001,"desc":"Give you a object reason"}



=== TEST 12: setup route with plugin
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"plugins": {
"opa": {
"host": "http://127.0.0.1:8181",
"policy": "example",
"send_headers_upstream": ["user"]
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uris": ["/echo"]
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- response_body
passed



=== TEST 13: hit route
--- request
GET /echo?test=1234&user=none
--- response_headers
user: none
Loading