From 5b344543c173cf31f468f8c2f17dea1154c7a6f2 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Thu, 3 Mar 2022 18:46:31 +0800 Subject: [PATCH 1/9] fix: rerun rewrite phase for newly added plugins in consumer Closes #6405 Signed-off-by: tzssangglass --- apisix/init.lua | 8 +- apisix/plugin.lua | 54 ++++++++- t/admin/consumer-plugins.t | 223 +++++++++++++++++++++++++++++++++++++ 3 files changed, 281 insertions(+), 4 deletions(-) create mode 100644 t/admin/consumer-plugins.t diff --git a/apisix/init.lua b/apisix/init.lua index 2f9befd4107b..fc72e3f3d371 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -454,8 +454,14 @@ function _M.http_access_phase() if changed then api_ctx.matched_route = route + local runned_plugins = core.table.deepcopy(api_ctx.plugins) core.table.clear(api_ctx.plugins) - api_ctx.plugins = plugin.filter(api_ctx, route, api_ctx.plugins) + local unrunn_plugins + api_ctx.plugins, unrunn_plugins = plugin.filter(api_ctx, route, api_ctx.plugins, nil, runned_plugins) + if unrunn_plugins then + -- rerun rewrite phase for newly added plugins in consumer + plugin.run_newly_added_plugins_in_consumer(unrunn_plugins, api_ctx) + end end end plugin.run_plugin("access", plugins, api_ctx) diff --git a/apisix/plugin.lua b/apisix/plugin.lua index 0d9b707f4552..af6de6cacd4d 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -355,7 +355,17 @@ local function trace_plugins_info_for_debug(ctx, plugins) end -function _M.filter(ctx, conf, plugins, route_conf) +local function filter_runned_plugins(runned_plugins) + local runned_plugins_names = core.table.new(#runned_plugins / 2, 0) + for i = 1, #runned_plugins, 2 do + core.table.insert(runned_plugins_names, runned_plugins[i].name) + end + + return runned_plugins_names +end + + +function _M.filter(ctx, conf, plugins, route_conf, runned_plugins) local user_plugin_conf = conf.value.plugins if user_plugin_conf == nil or core.table.nkeys(user_plugin_conf) == 0 then @@ -365,7 +375,14 @@ function _M.filter(ctx, conf, plugins, route_conf) return plugins or core.tablepool.fetch("plugins", 0, 0) end - local route_plugin_conf = route_conf and route_conf.value.plugins + local unrunn_plugins + local runned_plugins_names + if runned_plugins and type(runned_plugins) == "table" then + runned_plugins_names = filter_runned_plugins(runned_plugins) + unrunn_plugins = core.table.new(4, 0) + end + + local route_plugin_conf = route_conf and route_conf.value and route_conf.value.plugins plugins = plugins or core.tablepool.fetch("plugins", 32, 0) for _, plugin_obj in ipairs(local_plugins) do local name = plugin_obj.name @@ -379,6 +396,11 @@ function _M.filter(ctx, conf, plugins, route_conf) end end + if runned_plugins_names and not core.table.array_find(runned_plugins_names, name) then + core.table.insert(unrunn_plugins, plugin_obj) + core.table.insert(unrunn_plugins, plugin_conf) + end + core.table.insert(plugins, plugin_obj) core.table.insert(plugins, plugin_conf) @@ -388,7 +410,7 @@ function _M.filter(ctx, conf, plugins, route_conf) trace_plugins_info_for_debug(ctx, plugins) - return plugins + return plugins, unrunn_plugins end @@ -805,4 +827,30 @@ function _M.run_global_rules(api_ctx, global_rules, phase_name) end +function _M.run_newly_added_plugins_in_consumer(unrunn_plugins, api_ctx) + for i = 1, #unrunn_plugins, 2 do + -- only run rewrite phase of newly added plugins + local phase_func = unrunn_plugins[i]["rewrite"] + if phase_func then + local code, body = phase_func(unrunn_plugins[i + 1], api_ctx) + if code or body then + if is_http then + if code >= 400 then + core.log.warn(unrunn_plugins[i].name, " exits with http status code ", code) + end + + core.response.exit(code, body) + else + if code >= 400 then + core.log.warn(unrunn_plugins[i].name, " exits with status code ", code) + end + + ngx_exit(1) + end + end + end + end +end + + return _M diff --git a/t/admin/consumer-plugins.t b/t/admin/consumer-plugins.t new file mode 100644 index 000000000000..e073b9d4aaf0 --- /dev/null +++ b/t/admin/consumer-plugins.t @@ -0,0 +1,223 @@ +use t::APISIX 'no_plan'; + +log_level('info'); +repeat_each(1); +no_long_string(); +no_root_location(); + +add_block_preprocessor(sub { + my ($block) = @_; + + if (!$block->request) { + $block->set_value("request", "GET /t"); + } + + if (!$block->response_body) { + $block->set_value("response_body", "passed\n"); + } + + if (!$block->no_error_log && !$block->error_log) { + $block->set_value("no_error_log", "[error]\n[alert]"); + } +}); + + +our $debug_config = t::APISIX::read_file("conf/debug.yaml"); +$debug_config =~ s/basic:\n enable: false/basic:\n enable: true/; +$debug_config =~ s/hook_conf:\n enable: false/hook_conf:\n enable: true/; + +run_tests; + +__DATA__ + +=== TEST 1: configure non-auth plugins in the consumer and run ti's rewrite phase +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/consumers/jack', + ngx.HTTP_PUT, + [[{ + "username": "jack", + "plugins": { + "key-auth": { + "key": "auth-jack" + }, + "proxy-rewrite": { + "uri": "/uri/plugin_proxy_rewrite", + "headers": { + "X-Api-Engine": "APISIX", + "X-CONSUMER-ID": "1" + } + } + } + }]] + ) + + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "key-auth": {} + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 2: hit routes +--- request +GET /hello +--- more_headers +apikey: auth-jack +--- response_body +uri: /uri/plugin_proxy_rewrite +apikey: auth-jack +host: localhost +x-api-engine: APISIX +x-consumer-id: 1 +x-real-ip: 127.0.0.1 + + + +=== TEST 3: trace plugins info for debug +--- debug_config eval: $::debug_config +--- config + location /t { + content_by_lua_block { + local json = require("toolkit.json") + local ngx_re = require("ngx.re") + local http = require "resty.http" + local httpc = http.new() + local headers = {} + headers["apikey"] = "auth-jack" + local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" + local res, err = httpc:request_uri(uri, { + method = "GET", + headers = headers, + }) + local debug_header = res.headers["Apisix-Plugins"] + local arr = ngx_re.split(debug_header, ", ") + local hash = {} + for i, v in ipairs(arr) do + hash[v] = true + end + ngx.status = res.status + ngx.say(json.encode(hash)) + } + } +--- response_body +{"key-auth":true,"proxy-rewrite":true} + + + +=== TEST 4: configure non-auth plugins in the route and run ti's rewrite phase +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/consumers/jack', + ngx.HTTP_PUT, + [[{ + "username": "jack", + "plugins": { + "key-auth": { + "key": "auth-jack" + } + } + }]] + ) + + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "key-auth": {}, + "proxy-rewrite": { + "uri": "/uri/plugin_proxy_rewrite", + "headers": { + "X-Api-Engine": "APISIX", + "X-CONSUMER-ID": "1" + } + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 5: hit routes +--- request +GET /hello +--- more_headers +apikey: auth-jack +--- response_body +uri: /uri/plugin_proxy_rewrite +apikey: auth-jack +host: localhost +x-api-engine: APISIX +x-consumer-id: 1 +x-real-ip: 127.0.0.1 + + + +=== TEST 6: trace plugins info for debug +--- debug_config eval: $::debug_config +--- config + location /t { + content_by_lua_block { + local json = require("toolkit.json") + local ngx_re = require("ngx.re") + local http = require "resty.http" + local httpc = http.new() + local headers = {} + headers["apikey"] = "auth-jack" + local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" + local res, err = httpc:request_uri(uri, { + method = "GET", + headers = headers, + }) + local debug_header = res.headers["Apisix-Plugins"] + local arr = ngx_re.split(debug_header, ", ") + local hash = {} + for i, v in ipairs(arr) do + hash[v] = true + end + ngx.status = res.status + ngx.say(json.encode(hash)) + } + } +--- response_body +{"key-auth":true,"proxy-rewrite":true} From e1510495ec51a6b6d707efb86e3ba9629df4d451 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Thu, 3 Mar 2022 18:54:15 +0800 Subject: [PATCH 2/9] fix check Signed-off-by: tzssangglass --- apisix/init.lua | 3 ++- t/admin/consumer-plugins.t | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/apisix/init.lua b/apisix/init.lua index fc72e3f3d371..ce2d5aeba34a 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -457,7 +457,8 @@ function _M.http_access_phase() local runned_plugins = core.table.deepcopy(api_ctx.plugins) core.table.clear(api_ctx.plugins) local unrunn_plugins - api_ctx.plugins, unrunn_plugins = plugin.filter(api_ctx, route, api_ctx.plugins, nil, runned_plugins) + api_ctx.plugins, unrunn_plugins = plugin.filter(api_ctx, route, + api_ctx.plugins, nil, runned_plugins) if unrunn_plugins then -- rerun rewrite phase for newly added plugins in consumer plugin.run_newly_added_plugins_in_consumer(unrunn_plugins, api_ctx) diff --git a/t/admin/consumer-plugins.t b/t/admin/consumer-plugins.t index e073b9d4aaf0..fc4963d57037 100644 --- a/t/admin/consumer-plugins.t +++ b/t/admin/consumer-plugins.t @@ -1,3 +1,20 @@ +# +# 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'; log_level('info'); From 5b52e368826063494f49985f9db2c74d6e910685 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Thu, 3 Mar 2022 21:30:47 +0800 Subject: [PATCH 3/9] fix CI, only the call core.consumer.attach_consumer is considered to be an auth class plugin Signed-off-by: tzssangglass --- apisix/plugin.lua | 7 +++++-- t/{admin/consumer-plugins.t => node/consumer-plugin2.t} | 0 2 files changed, 5 insertions(+), 2 deletions(-) rename t/{admin/consumer-plugins.t => node/consumer-plugin2.t} (100%) diff --git a/apisix/plugin.lua b/apisix/plugin.lua index af6de6cacd4d..5b71adb2b2ae 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -397,8 +397,11 @@ function _M.filter(ctx, conf, plugins, route_conf, runned_plugins) end if runned_plugins_names and not core.table.array_find(runned_plugins_names, name) then - core.table.insert(unrunn_plugins, plugin_obj) - core.table.insert(unrunn_plugins, plugin_conf) + -- no need to rerun the auth plugins + if plugin_obj.type ~= 'auth' then + core.table.insert(unrunn_plugins, plugin_obj) + core.table.insert(unrunn_plugins, plugin_conf) + end end core.table.insert(plugins, plugin_obj) diff --git a/t/admin/consumer-plugins.t b/t/node/consumer-plugin2.t similarity index 100% rename from t/admin/consumer-plugins.t rename to t/node/consumer-plugin2.t From 3ff237c745a015abc17a58ec93a86174d26c1e6d Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Thu, 3 Mar 2022 22:04:06 +0800 Subject: [PATCH 4/9] optimize Signed-off-by: tzssangglass --- apisix/init.lua | 2 +- apisix/plugin.lua | 43 +++++-------------------------------------- 2 files changed, 6 insertions(+), 39 deletions(-) diff --git a/apisix/init.lua b/apisix/init.lua index ce2d5aeba34a..8451eb2e6fca 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -461,7 +461,7 @@ function _M.http_access_phase() api_ctx.plugins, nil, runned_plugins) if unrunn_plugins then -- rerun rewrite phase for newly added plugins in consumer - plugin.run_newly_added_plugins_in_consumer(unrunn_plugins, api_ctx) + plugin.run_plugin("rewrite", unrunn_plugins, api_ctx) end end end diff --git a/apisix/plugin.lua b/apisix/plugin.lua index 5b71adb2b2ae..e0b63f83ed03 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -355,17 +355,7 @@ local function trace_plugins_info_for_debug(ctx, plugins) end -local function filter_runned_plugins(runned_plugins) - local runned_plugins_names = core.table.new(#runned_plugins / 2, 0) - for i = 1, #runned_plugins, 2 do - core.table.insert(runned_plugins_names, runned_plugins[i].name) - end - - return runned_plugins_names -end - - -function _M.filter(ctx, conf, plugins, route_conf, runned_plugins) +function _M.filter(ctx, conf, plugins, route_conf) local user_plugin_conf = conf.value.plugins if user_plugin_conf == nil or core.table.nkeys(user_plugin_conf) == 0 then @@ -378,7 +368,10 @@ function _M.filter(ctx, conf, plugins, route_conf, runned_plugins) local unrunn_plugins local runned_plugins_names if runned_plugins and type(runned_plugins) == "table" then - runned_plugins_names = filter_runned_plugins(runned_plugins) + runned_plugins_names = core.table.new(#runned_plugins / 2, 0) + for i = 1, #runned_plugins, 2 do + core.table.insert(runned_plugins_names, runned_plugins[i].name) + end unrunn_plugins = core.table.new(4, 0) end @@ -830,30 +823,4 @@ function _M.run_global_rules(api_ctx, global_rules, phase_name) end -function _M.run_newly_added_plugins_in_consumer(unrunn_plugins, api_ctx) - for i = 1, #unrunn_plugins, 2 do - -- only run rewrite phase of newly added plugins - local phase_func = unrunn_plugins[i]["rewrite"] - if phase_func then - local code, body = phase_func(unrunn_plugins[i + 1], api_ctx) - if code or body then - if is_http then - if code >= 400 then - core.log.warn(unrunn_plugins[i].name, " exits with http status code ", code) - end - - core.response.exit(code, body) - else - if code >= 400 then - core.log.warn(unrunn_plugins[i].name, " exits with status code ", code) - end - - ngx_exit(1) - end - end - end - end -end - - return _M From fe7f4c493933630835f767f18c26403ae6237600 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Fri, 4 Mar 2022 09:29:49 +0800 Subject: [PATCH 5/9] fixed CI Signed-off-by: tzssangglass --- apisix/plugin.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apisix/plugin.lua b/apisix/plugin.lua index e0b63f83ed03..3c05a35c6e99 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -355,7 +355,7 @@ local function trace_plugins_info_for_debug(ctx, plugins) end -function _M.filter(ctx, conf, plugins, route_conf) +function _M.filter(ctx, conf, plugins, route_conf, runned_plugins) local user_plugin_conf = conf.value.plugins if user_plugin_conf == nil or core.table.nkeys(user_plugin_conf) == 0 then From 19de4b9dcfb80e7d11c584a56bf43ed967a43515 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Fri, 4 Mar 2022 09:32:20 +0800 Subject: [PATCH 6/9] fixed typo Signed-off-by: tzssangglass --- t/node/consumer-plugin2.t | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/node/consumer-plugin2.t b/t/node/consumer-plugin2.t index fc4963d57037..249441a6c1a4 100644 --- a/t/node/consumer-plugin2.t +++ b/t/node/consumer-plugin2.t @@ -47,7 +47,7 @@ run_tests; __DATA__ -=== TEST 1: configure non-auth plugins in the consumer and run ti's rewrite phase +=== TEST 1: configure non-auth plugins in the consumer and run it's rewrite phase --- config location /t { content_by_lua_block { @@ -144,7 +144,7 @@ x-real-ip: 127.0.0.1 -=== TEST 4: configure non-auth plugins in the route and run ti's rewrite phase +=== TEST 4: configure non-auth plugins in the route and run it's rewrite phase --- config location /t { content_by_lua_block { From 7b3ed5089e7138dbaaa5602746a8c38eccf1b8f2 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Fri, 4 Mar 2022 16:47:26 +0800 Subject: [PATCH 7/9] resolve review Signed-off-by: tzssangglass --- apisix/init.lua | 11 +++------ apisix/plugin.lua | 57 ++++++++++++++++++++++++++++++----------------- 2 files changed, 39 insertions(+), 29 deletions(-) diff --git a/apisix/init.lua b/apisix/init.lua index 8451eb2e6fca..904bd209761d 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -454,15 +454,10 @@ function _M.http_access_phase() if changed then api_ctx.matched_route = route - local runned_plugins = core.table.deepcopy(api_ctx.plugins) core.table.clear(api_ctx.plugins) - local unrunn_plugins - api_ctx.plugins, unrunn_plugins = plugin.filter(api_ctx, route, - api_ctx.plugins, nil, runned_plugins) - if unrunn_plugins then - -- rerun rewrite phase for newly added plugins in consumer - plugin.run_plugin("rewrite", unrunn_plugins, api_ctx) - end + api_ctx.plugins = plugin.filter(api_ctx, route, api_ctx.plugins) + -- rerun rewrite phase for newly added plugins in consumer + plugin.rerun_plugins_of_consumer(api_ctx.plugins, api_ctx) end end plugin.run_plugin("access", plugins, api_ctx) diff --git a/apisix/plugin.lua b/apisix/plugin.lua index 3c05a35c6e99..9fa9d83bdbb6 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -355,7 +355,7 @@ local function trace_plugins_info_for_debug(ctx, plugins) end -function _M.filter(ctx, conf, plugins, route_conf, runned_plugins) +function _M.filter(ctx, conf, plugins, route_conf) local user_plugin_conf = conf.value.plugins if user_plugin_conf == nil or core.table.nkeys(user_plugin_conf) == 0 then @@ -365,17 +365,7 @@ function _M.filter(ctx, conf, plugins, route_conf, runned_plugins) return plugins or core.tablepool.fetch("plugins", 0, 0) end - local unrunn_plugins - local runned_plugins_names - if runned_plugins and type(runned_plugins) == "table" then - runned_plugins_names = core.table.new(#runned_plugins / 2, 0) - for i = 1, #runned_plugins, 2 do - core.table.insert(runned_plugins_names, runned_plugins[i].name) - end - unrunn_plugins = core.table.new(4, 0) - end - - local route_plugin_conf = route_conf and route_conf.value and route_conf.value.plugins + local route_plugin_conf = route_conf and route_conf.value.plugins plugins = plugins or core.tablepool.fetch("plugins", 32, 0) for _, plugin_obj in ipairs(local_plugins) do local name = plugin_obj.name @@ -389,14 +379,6 @@ function _M.filter(ctx, conf, plugins, route_conf, runned_plugins) end end - if runned_plugins_names and not core.table.array_find(runned_plugins_names, name) then - -- no need to rerun the auth plugins - if plugin_obj.type ~= 'auth' then - core.table.insert(unrunn_plugins, plugin_obj) - core.table.insert(unrunn_plugins, plugin_conf) - end - end - core.table.insert(plugins, plugin_obj) core.table.insert(plugins, plugin_conf) @@ -406,7 +388,7 @@ function _M.filter(ctx, conf, plugins, route_conf, runned_plugins) trace_plugins_info_for_debug(ctx, plugins) - return plugins, unrunn_plugins + return plugins end @@ -521,6 +503,11 @@ local function merge_consumer_route(route_conf, consumer_conf) end new_route_conf.value.plugins[name] = conf + + if (route_conf and route_conf.value and route_conf.value.plugins) + and not route_conf.value.plugins[name] then + new_route_conf.value.plugins[name]["_un_running"] = true + end end core.log.info("merged conf : ", core.json.delay_encode(new_route_conf)) @@ -823,4 +810,32 @@ function _M.run_global_rules(api_ctx, global_rules, phase_name) end +function _M.rerun_plugins_of_consumer(plugins, api_ctx) + for i = 1, #plugins, 2 do + -- no need to rerun the auth plugins + if plugins[i + 1]["_un_running"] and plugins[i].type ~= "auth" then + local phase_func = plugins[i]["rewrite"] + if phase_func then + local code, body = phase_func(plugins[i + 1], api_ctx) + if code or body then + if is_http then + if code >= 400 then + core.log.warn(plugins[i].name, " exits with http status code ", code) + end + + core.response.exit(code, body) + else + if code >= 400 then + core.log.warn(plugins[i].name, " exits with status code ", code) + end + + ngx_exit(1) + end + end + end + end + end + return api_ctx +end + return _M From 8031ea70d7d7ba0d1eb4151d824b6384c4654d66 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Fri, 4 Mar 2022 16:57:20 +0800 Subject: [PATCH 8/9] fix typo Signed-off-by: tzssangglass --- apisix/plugin.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apisix/plugin.lua b/apisix/plugin.lua index 9fa9d83bdbb6..d66ec0458ee9 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -506,7 +506,7 @@ local function merge_consumer_route(route_conf, consumer_conf) if (route_conf and route_conf.value and route_conf.value.plugins) and not route_conf.value.plugins[name] then - new_route_conf.value.plugins[name]["_un_running"] = true + new_route_conf.value.plugins[name]["_from_consumer"] = true end end @@ -813,7 +813,7 @@ end function _M.rerun_plugins_of_consumer(plugins, api_ctx) for i = 1, #plugins, 2 do -- no need to rerun the auth plugins - if plugins[i + 1]["_un_running"] and plugins[i].type ~= "auth" then + if plugins[i + 1]["_from_consumer"] and plugins[i].type ~= "auth" then local phase_func = plugins[i]["rewrite"] if phase_func then local code, body = phase_func(plugins[i + 1], api_ctx) From 80e68759fb2f169c98bd88d4f9b45a9a76bebf39 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Mon, 7 Mar 2022 16:26:13 +0800 Subject: [PATCH 9/9] resolve code review Signed-off-by: tzssangglass --- apisix/init.lua | 2 +- apisix/plugin.lua | 40 +++++++--------------------------------- 2 files changed, 8 insertions(+), 34 deletions(-) diff --git a/apisix/init.lua b/apisix/init.lua index 904bd209761d..1298e1c20683 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -457,7 +457,7 @@ function _M.http_access_phase() core.table.clear(api_ctx.plugins) api_ctx.plugins = plugin.filter(api_ctx, route, api_ctx.plugins) -- rerun rewrite phase for newly added plugins in consumer - plugin.rerun_plugins_of_consumer(api_ctx.plugins, api_ctx) + plugin.run_plugin("rewrite_in_consumer", api_ctx.plugins, api_ctx) end end plugin.run_plugin("access", plugins, api_ctx) diff --git a/apisix/plugin.lua b/apisix/plugin.lua index d66ec0458ee9..0221e6c4e77a 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -502,12 +502,10 @@ local function merge_consumer_route(route_conf, consumer_conf) new_route_conf.value.plugins = {} end - new_route_conf.value.plugins[name] = conf - - if (route_conf and route_conf.value and route_conf.value.plugins) - and not route_conf.value.plugins[name] then - new_route_conf.value.plugins[name]["_from_consumer"] = true + if new_route_conf.value.plugins[name] == nil then + conf._from_consumer = true end + new_route_conf.value.plugins[name] = conf end core.log.info("merged conf : ", core.json.delay_encode(new_route_conf)) @@ -737,6 +735,10 @@ function _M.run_plugin(phase, plugins, api_ctx) and phase ~= "body_filter" then for i = 1, #plugins, 2 do + if phase == "rewrite_in_consumer" and plugins[i + 1]._from_consumer + and plugins[i].type ~= "auth"then + phase = "rewrite" + end local phase_func = plugins[i][phase] if phase_func then plugin_run = true @@ -810,32 +812,4 @@ function _M.run_global_rules(api_ctx, global_rules, phase_name) end -function _M.rerun_plugins_of_consumer(plugins, api_ctx) - for i = 1, #plugins, 2 do - -- no need to rerun the auth plugins - if plugins[i + 1]["_from_consumer"] and plugins[i].type ~= "auth" then - local phase_func = plugins[i]["rewrite"] - if phase_func then - local code, body = phase_func(plugins[i + 1], api_ctx) - if code or body then - if is_http then - if code >= 400 then - core.log.warn(plugins[i].name, " exits with http status code ", code) - end - - core.response.exit(code, body) - else - if code >= 400 then - core.log.warn(plugins[i].name, " exits with status code ", code) - end - - ngx_exit(1) - end - end - end - end - end - return api_ctx -end - return _M