fix: disallow creating duplicate plugins in global rules (#12800)

Author: shreemaan-abhishek

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
 

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)

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)