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.

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

feat: add public api plugin #6145

Merged
merged 17 commits into from
Jan 24, 2022
Merged
24 changes: 13 additions & 11 deletions apisix/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -373,21 +373,23 @@ function _M.http_access_phase()
api_ctx.var.real_request_uri = api_ctx.var.request_uri
api_ctx.var.request_uri = api_ctx.var.uri .. api_ctx.var.is_args .. (api_ctx.var.args or "")

if router.api.has_route_not_under_apisix() or
core.string.has_prefix(uri, "/apisix/")
then
local skip = local_conf and local_conf.apisix.global_rule_skip_internal_api
local matched = router.api.match(api_ctx, skip)
if matched then
return
end
end

router.router_http.match(api_ctx)

local route = api_ctx.matched_route
if not route then
-- run global rule
-- whether the public API run global rules is
-- controlled by the configuration file
if router.api.has_route_not_under_apisix() or
core.string.has_prefix(uri, "/apisix/")
then
local skip = local_conf and local_conf.apisix.global_rule_skip_internal_api
local matched = router.api.match(api_ctx, skip)
if matched then
return
end
end

-- run global rule when there is no matching route
plugin.run_global_rules(api_ctx, router.global_rules, nil)

core.log.info("not find any matched route")
Expand Down
58 changes: 58 additions & 0 deletions apisix/plugins/public-api.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
--
-- Licensed to the Apache Software Foundation (ASF) under one or more
-- contributor license agreements. See the NOTICE file distributed with
-- this work for additional information regarding copyright ownership.
-- The ASF licenses this file to You under the Apache License, Version 2.0
-- (the "License"); you may not use this file except in compliance with
-- the License. You may obtain a copy of the License at
--
-- http://www.apache.org/licenses/LICENSE-2.0
--
-- Unless required by applicable law or agreed to in writing, software
-- distributed under the License is distributed on an "AS IS" BASIS,
-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-- See the License for the specific language governing permissions and
-- limitations under the License.
--

local core = require("apisix.core")
local router = require("apisix.router")

local schema = {
type = "object",
properties = {
uri = {type = "string"},
},
}


local _M = {
version = 0.1,
priority = 501,
name = "public-api",
schema = schema,
}


function _M.check_schema(conf)
return core.schema.check(schema, conf)
end


function _M.access(conf, ctx)
local local_conf = core.config.local_conf()

-- overwrite the uri in the ctx when the user has set the target uri
ctx.var.uri = conf.uri or ctx.var.uri
local skip = local_conf and local_conf.apisix.global_rule_skip_internal_api
Copy link
Member

Choose a reason for hiding this comment

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

We don't need to run global rules twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

apisix/apisix/init.lua

Lines 376 to 445 in 64c47df

router.router_http.match(api_ctx)
local route = api_ctx.matched_route
if not route then
-- whether the public API run global rules is
-- controlled by the configuration file
if router.api.has_route_not_under_apisix() or
core.string.has_prefix(uri, "/apisix/")
then
local skip = local_conf and local_conf.apisix.global_rule_skip_internal_api
local matched = router.api.match(api_ctx, skip)
if matched then
return
end
end
-- run global rule when there is no matching route
plugin.run_global_rules(api_ctx, router.global_rules, nil)
core.log.info("not find any matched route")
return core.response.exit(404,
{error_msg = "404 Route Not Found"})
end
core.log.info("matched route: ",
core.json.delay_encode(api_ctx.matched_route, true))
local enable_websocket = route.value.enable_websocket
if route.value.plugin_config_id then
local conf = plugin_config.get(route.value.plugin_config_id)
if not conf then
core.log.error("failed to fetch plugin config by ",
"id: ", route.value.plugin_config_id)
return core.response.exit(503)
end
route = plugin_config.merge(route, conf)
end
if route.value.service_id then
local service = service_fetch(route.value.service_id)
if not service then
core.log.error("failed to fetch service configuration by ",
"id: ", route.value.service_id)
return core.response.exit(404)
end
route = plugin.merge_service_route(service, route)
api_ctx.matched_route = route
api_ctx.conf_type = "route&service"
api_ctx.conf_version = route.modifiedIndex .. "&" .. service.modifiedIndex
api_ctx.conf_id = route.value.id .. "&" .. service.value.id
api_ctx.service_id = service.value.id
api_ctx.service_name = service.value.name
if enable_websocket == nil then
enable_websocket = service.value.enable_websocket
end
else
api_ctx.conf_type = "route"
api_ctx.conf_version = route.modifiedIndex
api_ctx.conf_id = route.value.id
end
api_ctx.route_id = route.value.id
api_ctx.route_name = route.value.name
-- run global rule
plugin.run_global_rules(api_ctx, router.global_rules, nil)

Hi, @spacewander.

Based on this part of the code, we can presume to have such a situation.

  1. HTTP match, and public-api plugin is enabled.
    This will be controlled by the configuration file whether to enforce global rules for public API calls. After the match, the subsequent request processing flow will be completely taken over by the public API handler, bypassing the rest of the code execution in the access phase.
  2. HTTP not matched, public API route matched.
    This will also be controlled by the configuration file whether to enforce global rules or not.
  3. Both HTTP and public APIs do not match
    It will force the global rule to run and return 404 Not Route.

Also, as you said in this issue #6137 (comment), we will adjust the public API to not be exposed by default, which will be done by completely removing the public API route match in the access phase. (Currently HTTP matching is performed followed by public API matching as a backup) In that case, the matching and processing of the public API will be done by the public-api plugin only, and if a match is made the global rules will be enforced according to the configuration.

Copy link
Member

Choose a reason for hiding this comment

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

In that case, the matching and processing of the public API will be done by the public-api plugin only, and if a match is made the global rules will be enforced according to the configuration.

We can adjust it when we retire the router? The current implementation will run global rules twice in some cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try to fix it.

Copy link
Contributor Author

@bzp2010 bzp2010 Jan 23, 2022

Choose a reason for hiding this comment

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

Update

After checking again, I think there may not be a duplicate calls problem.

Details

image

There are 4 calls to run_global_rules executed in the current code. One of them is in api_router, which is controlled by the configuration file (this is the router used by the public API), and the remaining ones are executed in the http_access_phase of init.

  1. both HTTP and public API do not match, will run and return a 404 status code, stop progress
    image

  2. run after HTTP matched
    image

  3. run in common_phase for before_proxy

In fact I didn't add global_rules to init.lua, they are still being executed according to the original logic.

If I have missed anything, please help point it out. Thanks.

ping @spacewander


-- perform route matching
if router.api.match(ctx, skip) then
return
end

return 404
end


return _M
1 change: 1 addition & 0 deletions conf/config-default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ plugins: # plugin list (sorted by priority)
#- dubbo-proxy # priority: 507
- grpc-transcode # priority: 506
- grpc-web # priority: 505
- public-api # priority: 501
- prometheus # priority: 500
- datadog # priority: 495
- echo # priority: 412
Expand Down
1 change: 1 addition & 0 deletions t/admin/plugins.t
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ redirect
response-rewrite
grpc-transcode
grpc-web
public-api
prometheus
datadog
echo
Expand Down
15 changes: 15 additions & 0 deletions t/plugin/plugin.t
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,21 @@ passed
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test

local code, err = t('/apisix/admin/routes/jwt',
ngx.HTTP_PUT,
[[{
"uri": "/apisix/plugin/jwt/sign",
"plugins": { "public-api": {} }
}]]
)

if code >= 300 then
ngx.status = code
ngx.say(err)
return
end

local code, err, sign = t('/apisix/plugin/jwt/sign?key=user-key',
ngx.HTTP_GET
)
Expand Down
219 changes: 219 additions & 0 deletions t/plugin/public-api.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
#
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
use t::APISIX 'no_plan';

repeat_each(1);
no_long_string();
no_root_location();

add_block_preprocessor(sub {
my ($block) = @_;

if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
$block->set_value("no_error_log", "[error]");
}

if (!defined $block->request) {
$block->set_value("request", "GET /t");
}
});

run_tests();

__DATA__

=== TEST 1: sanity
--- config
location /t {
content_by_lua_block {
local test_cases = {
{uri = "/apisix/plugin/jwt/sign"},
{uri = 3233}
}
local plugin = require("apisix.plugins.public-api")

for _, case in ipairs(test_cases) do
local ok, err = plugin.check_schema(case)
ngx.say(ok and "done" or err)
end
}
}
--- response_body
done
property "uri" validation failed: wrong type: expected string, got number



=== TEST 2: set route
--- config
location /t {
content_by_lua_block {
local datas = {
{
uri = "/apisix/admin/consumers",
data = [[{
"username": "alice",
"plugins": {
"jwt-auth": {
"key": "user-key",
"algorithm": "HS256"
}
}
}]]
},
{
uri = "/apisix/admin/routes/custom-jwt-sign",
data = [[{
"plugins": {
"public-api": {
"uri": "/apisix/plugin/jwt/sign"
},
"serverless-pre-function": {
"phase": "rewrite",
"functions": ["return function(conf, ctx) require(\"apisix.core\").log.warn(\"custom-jwt-sign was triggered\"); end"]
}
},
"uri": "/gen_token"
}]],
},
{
uri = "/apisix/admin/routes/direct-wolf-rbac-userinfo",
data = [[{
"plugins": {
"public-api": {},
"serverless-pre-function": {
"phase": "rewrite",
"functions": ["return function(conf, ctx) require(\"apisix.core\").log.warn(\"direct-wolf-rbac-userinfo was triggered\"); end"]
}
},
"uri": "/apisix/plugin/wolf-rbac/user_info"
}]],
},
}

local t = require("lib.test_admin").test

for _, data in ipairs(datas) do
local code, body = t(data.uri, ngx.HTTP_PUT, data.data)
ngx.say(code..body)
end
}
}
--- response_body eval
"201passed\n" x 3



=== TEST 3: hit route (custom-jwt-sign)
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test

local code, body, jwt = t("/gen_token?key=user-key", ngx.HTTP_GET, "", nil, {apikey = "testkey"})
if code >= 300 then
ngx.status = code
end

local header = string.sub(jwt, 1, 36)

if header == "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9" or
header == "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9" then
ngx.say("passed")
return
end

ngx.say("failed")
}
}
--- response_body
passed



=== TEST 4: hit route (direct-wolf-rbac-userinfo)
--- request
GET /apisix/plugin/wolf-rbac/user_info
--- error_code: 401
--- error_log
direct-wolf-rbac-userinfo was triggered



=== TEST 5: missing route
--- request
GET /apisix/plugin/balalbala
--- error_code: 404
Copy link
Member

Choose a reason for hiding this comment

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

Let's add more test that the uri in public API plugin matches nothing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it, adding

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added




=== TEST 6: setup route (protect public API)
--- config
location /t {
content_by_lua_block {
local datas = {
{
uri = "/apisix/admin/consumers",
data = [[{
"username": "bob",
"plugins": {
"key-auth": {
"key": "testkey"
}
}
}]]
},
{
uri = "/apisix/admin/routes/custom-jwt-sign",
data = [[{
"plugins": {
"public-api": {
"uri": "/apisix/plugin/jwt/sign"
},
"key-auth": {}
},
"uri": "/gen_token"
}]],
}
}

local t = require("lib.test_admin").test

for _, data in ipairs(datas) do
local code, body = t(data.uri, ngx.HTTP_PUT, data.data)
ngx.say(code..body)
end
}
}
--- response_body
201passed
200passed



=== TEST 7: hit route (with key-auth header)
--- request
GET /gen_token?key=user-key
--- more_headers
apikey: testkey



=== TEST 8: hit route (without key-auth header)
--- request
GET /gen_token?key=user-key
--- error_code: 401