This is an automated email from the ASF dual-hosted git repository.
shreemaanabhishek pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix.git
The following commit(s) were added to refs/heads/master by this push:
new 3af575914 fix: disallow creating duplicate plugins in global rules
(#12800)
3af575914 is described below
commit 3af5759149524230b722819a4068cf750561eef2
Author: Shreemaan Abhishek <[email protected]>
AuthorDate: Thu Jan 8 18:49:56 2026 +0545
fix: disallow creating duplicate plugins in global rules (#12800)
---
apisix/admin/global_rules.lua | 37 +++++
apisix/init.lua | 11 +-
apisix/plugin.lua | 76 ++++++++--
t/admin/global-rules-conflict.t | 298 ++++++++++++++++++++++++++++++++++++++++
t/node/global-rule.t | 51 +------
t/plugin/plugin.t | 63 ++-------
6 files changed, 418 insertions(+), 118 deletions(-)
diff --git a/apisix/admin/global_rules.lua b/apisix/admin/global_rules.lua
index 3531a6c7e..0abd5926b 100644
--- a/apisix/admin/global_rules.lua
+++ b/apisix/admin/global_rules.lua
@@ -19,6 +19,17 @@ local resource = require("apisix.admin.resource")
local schema_plugin = require("apisix.admin.plugins").check_schema
local plugins_encrypt_conf = require("apisix.admin.plugins").encrypt_conf
+local pairs = pairs
+local ipairs = ipairs
+local tostring = tostring
+
+local function get_global_rules()
+ local g = core.etcd.get("/global_rules", true)
+ if not g then
+ return nil
+ end
+ return core.table.try_read_attr(g, "body", "list")
+end
local function check_conf(id, conf, need_id, schema)
local ok, err = core.schema.check(schema, conf)
@@ -31,6 +42,32 @@ local function check_conf(id, conf, need_id, schema)
return nil, {error_msg = err}
end
+ -- Check for plugin conflicts with existing global rules
+ if conf.plugins then
+ local global_rules = get_global_rules()
+ if global_rules then
+ for _, existing_rule in ipairs(global_rules) do
+ -- Skip checking against itself when updating
+ if existing_rule.value and existing_rule.value.id and
+ tostring(existing_rule.value.id) ~= tostring(id) then
+
+ if existing_rule.value.plugins then
+ -- Check for any overlapping plugins
+ for plugin_name, _ in pairs(conf.plugins) do
+ if existing_rule.value.plugins[plugin_name] then
+ return nil, {
+ error_msg = "plugin '" .. plugin_name ..
+ "' already exists in global rule with id
'" ..
+ existing_rule.value.id .. "'"
+ }
+ end
+ end
+ end
+ end
+ end
+ end
+ end
+
return true
end
diff --git a/apisix/init.lua b/apisix/init.lua
index 430572e27..e1aedabd2 100644
--- a/apisix/init.lua
+++ b/apisix/init.lua
@@ -468,7 +468,8 @@ local function common_phase(phase_name)
return
end
- plugin.run_global_rules(api_ctx, api_ctx.global_rules, phase_name)
+ local global_rules, conf_version = apisix_global_rules.global_rules()
+ plugin.run_global_rules(api_ctx, global_rules, conf_version, phase_name)
if api_ctx.script_obj then
script.run(phase_name, api_ctx)
@@ -721,8 +722,8 @@ function _M.http_access_phase()
local route = api_ctx.matched_route
if not route then
-- run global rule when there is no matching route
- local global_rules = apisix_global_rules.global_rules()
- plugin.run_global_rules(api_ctx, global_rules, nil)
+ local global_rules, conf_version = apisix_global_rules.global_rules()
+ plugin.run_global_rules(api_ctx, global_rules, conf_version, nil)
core.log.info("not find any matched route")
return core.response.exit(404,
@@ -774,8 +775,8 @@ function _M.http_access_phase()
api_ctx.route_name = route.value.name
-- run global rule
- local global_rules = apisix_global_rules.global_rules()
- plugin.run_global_rules(api_ctx, global_rules, nil)
+ local global_rules, conf_version = apisix_global_rules.global_rules()
+ plugin.run_global_rules(api_ctx, global_rules, conf_version, nil)
if route.value.script then
script.load(route, api_ctx)
diff --git a/apisix/plugin.lua b/apisix/plugin.lua
index 6d824e130..a8faccf03 100644
--- a/apisix/plugin.lua
+++ b/apisix/plugin.lua
@@ -59,6 +59,10 @@ local expr_lrucache = core.lrucache.new({
local meta_pre_func_load_lrucache = core.lrucache.new({
ttl = 300, count = 512
})
+local merge_global_rule_lrucache = core.lrucache.new({
+ ttl = 300, count = 512
+})
+
local local_conf
local check_plugin_metadata
@@ -1263,7 +1267,45 @@ function _M.set_plugins_meta_parent(plugins, parent)
end
-function _M.run_global_rules(api_ctx, global_rules, phase_name)
+local function merge_global_rules(global_rules, conf_version)
+ -- First pass: identify duplicate plugins across all global rules
+ local plugins_hash = {}
+ local seen_plugin = {}
+ local values = global_rules
+ for _, global_rule in config_util.iterate_values(values) do
+ if global_rule.value and global_rule.value.plugins then
+ for plugin_name, plugin_conf in pairs(global_rule.value.plugins) do
+ if seen_plugin[plugin_name] then
+ core.log.error("Found ", plugin_name,
+ " configured across different global rules.",
+ " Removing it from execution list")
+ plugins_hash[plugin_name] = nil
+ else
+ plugins_hash[plugin_name] = plugin_conf
+ seen_plugin[plugin_name] = true
+ end
+ end
+ end
+ end
+
+ local dummy_global_rule = {
+ key = "/apisix/global_rules/dummy",
+ value = {
+ updated_time = ngx.time(),
+ plugins = plugins_hash,
+ created_time = ngx.time(),
+ id = 1,
+ },
+ createdIndex = conf_version,
+ modifiedIndex = conf_version,
+ clean_handlers = {},
+ }
+
+ return dummy_global_rule
+end
+
+
+function _M.run_global_rules(api_ctx, global_rules, conf_version, phase_name)
if global_rules and #global_rules > 0 then
local orig_conf_type = api_ctx.conf_type
local orig_conf_version = api_ctx.conf_version
@@ -1273,22 +1315,26 @@ function _M.run_global_rules(api_ctx, global_rules,
phase_name)
api_ctx.global_rules = global_rules
end
+ local dummy_global_rule = merge_global_rule_lrucache(conf_version,
+ global_rules,
+
merge_global_rules,
+ global_rules,
+ conf_version)
+
local plugins = core.tablepool.fetch("plugins", 32, 0)
- local values = global_rules
local route = api_ctx.matched_route
- for _, global_rule in config_util.iterate_values(values) do
- api_ctx.conf_type = "global_rule"
- api_ctx.conf_version = global_rule.modifiedIndex
- api_ctx.conf_id = global_rule.value.id
-
- core.table.clear(plugins)
- plugins = _M.filter(api_ctx, global_rule, plugins, route)
- if phase_name == nil then
- _M.run_plugin("rewrite", plugins, api_ctx)
- _M.run_plugin("access", plugins, api_ctx)
- else
- _M.run_plugin(phase_name, plugins, api_ctx)
- end
+ api_ctx.conf_type = "global_rule"
+ api_ctx.conf_version = dummy_global_rule.modifiedIndex
+ api_ctx.conf_id = dummy_global_rule.value.id
+
+ core.table.clear(plugins)
+ plugins = _M.filter(api_ctx, dummy_global_rule, plugins, route)
+
+ if phase_name == nil then
+ _M.run_plugin("rewrite", plugins, api_ctx)
+ _M.run_plugin("access", plugins, api_ctx)
+ else
+ _M.run_plugin(phase_name, plugins, api_ctx)
end
core.tablepool.release("plugins", plugins)
diff --git a/t/admin/global-rules-conflict.t b/t/admin/global-rules-conflict.t
new file mode 100644
index 000000000..b3a3ccc34
--- /dev/null
+++ b/t/admin/global-rules-conflict.t
@@ -0,0 +1,298 @@
+#
+# 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();
+no_shuffle();
+log_level("info");
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: create first global rule with limit-count plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/global_rules/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 60,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ }
+ }]]
+ )
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- error_code: 201
+
+
+
+=== TEST 2: try to create second global rule with same plugin (should fail)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/global_rules/2',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-count": {
+ "count": 5,
+ "time_window": 120,
+ "rejected_code": 429,
+ "key": "remote_addr"
+ }
+ }
+ }]]
+ )
+ ngx.status = code
+ ngx.print(body)
+ }
+ }
+--- request
+GET /t
+--- response_body_like eval
+qr/plugin 'limit-count' already exists in global rule with id '1'/
+--- error_code: 400
+
+
+
+=== TEST 3: create second global rule with different plugin (should succeed)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/global_rules/2',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "ip-restriction": {
+ "whitelist": ["127.0.0.0/24"]
+ }
+ }
+ }]]
+ )
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- error_code: 201
+
+
+
+=== TEST 4: try to create third global rule with plugin from first rule
(should fail)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/global_rules/3',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-count": {
+ "count": 10,
+ "time_window": 30,
+ "rejected_code": 429,
+ "key": "remote_addr"
+ }
+ }
+ }]]
+ )
+ ngx.status = code
+ ngx.print(body)
+ }
+ }
+--- request
+GET /t
+--- response_body_like eval
+qr/plugin 'limit-count' already exists in global rule with id '1'/
+--- error_code: 400
+
+
+
+=== TEST 5: try to create global rule with multiple plugins where one
conflicts (should fail)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/global_rules/3',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-req": {
+ "rate": 1,
+ "burst": 0,
+ "key": "remote_addr"
+ },
+ "ip-restriction": {
+ "whitelist": ["192.168.0.0/16"]
+ }
+ }
+ }]]
+ )
+ ngx.status = code
+ ngx.print(body)
+ }
+ }
+--- request
+GET /t
+--- response_body_like eval
+qr/plugin 'ip-restriction' already exists in global rule with id '2'/
+--- error_code: 400
+
+
+
+=== TEST 6: update existing global rule with its current plugin (should
succeed)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/global_rules/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-count": {
+ "count": 3,
+ "time_window": 90,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ }
+ }]]
+ )
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- error_code: 200
+
+
+
+=== TEST 7: prepare data to test removal (during global rule execution) of
global rules with re-occurring plugins
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local etcd = require("apisix.core.etcd")
+ local http = require("resty.http")
+
+ local plugin_conf = function(id_val, count_val, t_val)
+ return {
+ id = id_val,
+ create_time = 1764938795,
+ update_time = 1764938811,
+ plugins = {
+ ["limit-count"] = {
+ key_type = "var",
+ sync_interval = -1,
+ show_limit_quota_header = true,
+ rejected_code = 503,
+ policy = "local",
+ key = "remote_addr",
+ allow_degradation = false,
+ count = count_val,
+ time_window = t_val
+ }
+ }
+ }
+ end
+
+ etcd.set("/global_rules/1", plugin_conf(1, 2, 10))
+ etcd.set("/global_rules/2", plugin_conf(2, 3, 60))
+ etcd.set("/global_rules/3", plugin_conf(3, 5, 20))
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "methods": ["GET"],
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ return
+ end
+
+ ngx.say("passed")
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- error_code: 200
+
+
+
+=== TEST 8: re-occuring plugins in global rules should be removed and not
executed
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local http = require("resty.http")
+
+
+ local httpc = http.new()
+ local res, err = httpc:request_uri("http://127.0.0.1:"; ..
ngx.var.server_port .. "/hello")
+
+ if not res then
+ ngx.say(err)
+ return
+ end
+
+ ngx.say("passed")
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- error_code: 200
+--- no_error_log
+limit key: global_rule
+--- error_log
+Found limit-count configured across different global rules. Removing it from
execution list
diff --git a/t/node/global-rule.t b/t/node/global-rule.t
index 0e61d68d8..1301a17c6 100644
--- a/t/node/global-rule.t
+++ b/t/node/global-rule.t
@@ -155,48 +155,16 @@ passed
-=== TEST 8: set one more global rule
---- config
- location /t {
- content_by_lua_block {
- local t = require("lib.test_admin").test
- local code, body = t('/apisix/admin/global_rules/2',
- ngx.HTTP_PUT,
- [[{
- "plugins": {
- "response-rewrite": {
- "headers": {
- "X-TEST":"test"
- }
- }
- }
- }]]
- )
-
- if code >= 300 then
- ngx.status = code
- end
- ngx.say(body)
- }
- }
---- request
-GET /t
---- response_body
-passed
-
-
-
-=== TEST 9: hit global rules
+=== TEST 8: hit global rules
--- request
GET /hello?name=;union%20select%20
--- error_code: 403
--- response_headers
X-VERSION: 1.0
-X-TEST: test
-=== TEST 10: hit global rules by internal api (only check uri-blocker)
+=== TEST 9: hit global rules by internal api (only check uri-blocker)
--- yaml_config
plugins:
- response-rewrite
@@ -207,11 +175,10 @@ GET /apisix/status?name=;union%20select%20
--- error_code: 403
--- response_headers
X-VERSION: 1.0
-X-TEST: test
-=== TEST 11: delete global rules
+=== TEST 10: delete global rules
--- config
location /t {
content_by_lua_block {
@@ -223,12 +190,6 @@ X-TEST: test
end
ngx.say(body)
- local code, body = t('/apisix/admin/global_rules/2',
ngx.HTTP_DELETE)
-
- if code >= 300 then
- ngx.status = code
- end
-
local code, body = t('/not_found', ngx.HTTP_GET)
ngx.say(code)
local code, body = t('/not_found', ngx.HTTP_GET)
@@ -244,7 +205,7 @@ passed
-=== TEST 12: empty global rule
+=== TEST 11: empty global rule
--- config
location /t {
content_by_lua_block {
@@ -295,7 +256,7 @@ passed
-=== TEST 13: hit global rules
+=== TEST 12: hit global rules
--- request
GET /hello
--- response_body
@@ -303,7 +264,7 @@ changed
-=== TEST 14: global rule works with the consumer, after deleting the global
rule, ensure no stale plugins remaining
+=== TEST 13: global rule works with the consumer, after deleting the global
rule, ensure no stale plugins remaining
--- config
location /t {
content_by_lua_block {
diff --git a/t/plugin/plugin.t b/t/plugin/plugin.t
index 53c87b0b5..701c800d3 100644
--- a/t/plugin/plugin.t
+++ b/t/plugin/plugin.t
@@ -504,50 +504,7 @@ x-api-version: v4
-=== TEST 19: different global_rules with the same plugin will not use the same
meta.filter cache
---- config
- location /t {
- content_by_lua_block {
- local t = require("lib.test_admin").test
- local code, body = t('/apisix/admin/global_rules/3',
- ngx.HTTP_PUT,
- {
- plugins = {
- ["proxy-rewrite"] = {
- _meta = {
- filter = {
- {"arg_version", "==", "v5"}
- }
- },
- uri = "/echo",
- headers = {
- ["X-Api-Version"] = "v5"
- }
- }
- }
- }
- )
- if code >= 300 then
- ngx.print(body)
- else
- ngx.say(body)
- end
- }
- }
---- response_body
-passed
-
-
-
-=== TEST 20: hit global_rules which has the same plugin with different
meta.filter
---- pipelined_requests eval
-["GET /hello1?version=v4", "GET /hello1?version=v5"]
---- response_headers eval
-["x-api-version: v4", "x-api-version: v5"]
-
-
-
-=== TEST 21: use _meta.filter in response-rewrite plugin
+=== TEST 19: use _meta.filter in response-rewrite plugin
--- config
location /t {
content_by_lua_block {
@@ -590,7 +547,7 @@ passed
-=== TEST 22: upstream_status = 502, enable response-rewrite plugin
+=== TEST 20: upstream_status = 502, enable response-rewrite plugin
--- request
GET /specific_status
--- more_headers
@@ -601,7 +558,7 @@ test-header: error
-=== TEST 23: upstream_status = 200, disable response-rewrite plugin
+=== TEST 21: upstream_status = 200, disable response-rewrite plugin
--- request
GET /hello
--- response_headers
@@ -609,7 +566,7 @@ GET /hello
-=== TEST 24: use _meta.filter in response-rewrite plugin
+=== TEST 22: use _meta.filter in response-rewrite plugin
--- config
location /t {
content_by_lua_block {
@@ -653,20 +610,20 @@ passed
-=== TEST 25: proxy-rewrite plugin will set $http_foo_age, response-rewrite
plugin return 403
+=== TEST 23: proxy-rewrite plugin will set $http_foo_age, response-rewrite
plugin return 403
--- request
GET /hello?age=18
--- error_code: 403
-=== TEST 26: response-rewrite plugin disable, return 200
+=== TEST 24: response-rewrite plugin disable, return 200
--- request
GET /hello
-=== TEST 27: use response var in meta.filter
+=== TEST 25: use response var in meta.filter
--- config
location /t {
content_by_lua_block {
@@ -708,7 +665,7 @@ passed
-=== TEST 28: hit route: disable proxy-rewrite plugin
+=== TEST 26: hit route: disable proxy-rewrite plugin
--- request
GET /hello
--- response_headers
@@ -716,7 +673,7 @@ GET /hello
-=== TEST 29: use APISIX's built-in variables in meta.filter
+=== TEST 27: use APISIX's built-in variables in meta.filter
--- config
location /t {
content_by_lua_block {
@@ -757,7 +714,7 @@ passed
-=== TEST 30: hit route: proxy-rewrite enable with post_arg_xx in meta.filter
+=== TEST 28: hit route: proxy-rewrite enable with post_arg_xx in meta.filter
--- request
POST /hello
key=abc
