diff --git a/apisix/admin/global_rules.lua b/apisix/admin/global_rules.lua index 3531a6c7e0f4..0abd5926b6f9 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 430572e27e48..e1aedabd215e 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 6d824e130e86..a8faccf03548 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 000000000000..b3a3ccc34651 --- /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 0e61d68d873c..1301a17c6f56 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 53c87b0b5133..701c800d34f4 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