From 39e6ca0059046534447cfebfeb4c98a322f99de7 Mon Sep 17 00:00:00 2001 From: Jairo Laupa Date: Fri, 17 Jul 2026 09:52:17 +0200 Subject: [PATCH 1/3] perf(consumer): incremental consumer plugin tree rebuild instead of O(N) full rebuild per change _M.plugin() rebuilt the entire consumer plugin tree via core.lrucache.global on every consumers.conf_version change, which is O(N) on the request path. With a large consumer base and steady consumer churn, rebuilds take seconds and stack up: request latency collapses and CPU saturates. Replace the full rebuild with an incremental one: a pending-set fed by the etcd filter callback applies only changed consumers on conf_version change; a full rebuild happens only on bootstrap (first request); a lightweight 30s background consistency check (count compare + id scan) triggers a full rebuild only if it detects a discrepancy (covers the simultaneous create+delete edge). No config or API changes; consumers_count_for_lrucache is unchanged. --- apisix/consumer.lua | 286 +++++++++++++++++++++++- t/node/consumer-incremental-rebuild.t | 302 ++++++++++++++++++++++++++ 2 files changed, 580 insertions(+), 8 deletions(-) create mode 100644 t/node/consumer-incremental-rebuild.t diff --git a/apisix/consumer.lua b/apisix/consumer.lua index 9e3c2495c190..6e980d93f333 100644 --- a/apisix/consumer.lua +++ b/apisix/consumer.lua @@ -89,12 +89,13 @@ local function filter_consumers_list(data_list) end local plugin_consumer +local construct_consumer_data do local consumers_id_lrucache = core.lrucache.new({ count = consumers_count_for_lrucache }) -local function construct_consumer_data(val, name, plugin_config) +function construct_consumer_data(val, name, plugin_config) -- if the val is a Consumer, clone it to the local consumer; -- if the val is a Credential, to get the Consumer by consumer_name and then clone -- it to the local consumer. @@ -190,6 +191,200 @@ end end +-- Incremental consumer plugin tree rebuild. +-- Instead of rebuilding the full O(N) tree on every conf_version change, +-- only process consumers that were created/updated/deleted. +-- Full rebuild runs only on bootstrap (first request). +-- A background timer runs a lightweight consistency check every 30s: +-- it compares tracked IDs against consumers.values without reconstructing data. +-- Only triggers a full rebuild if it finds stale entries (extremely rare). +-- Motivation: stock APISIX rebuilds this whole O(N) tree on every conf_version change. + +local cached_plugins +local cached_conf_version = 0 +local node_index = {} -- "plugin\0eid" -> position in nodes array +local entry_plugins = {} -- eid -> { [plugin_name] = true } +local pending_set = {} -- eid -> consumer item (from filter callback) +local has_pending = false +local pending_delete = false -- a delete event was seen; reconcile removed ids +local tracked_count = 0 -- count of consumers.values at last sync +local FULL_SYNC_INTERVAL = 30 -- seconds between background consistency checks + +-- key-auth (and the other auth plugins) look consumers up by a key value, not by +-- position, via _M.consumers_kv() -> a per-plugin {key_value -> consumer} map. +-- That map was rebuilt in full on every conf_version change (O(N) over all +-- consumers, each cloned through a fill lru). We instead maintain it incrementally +-- alongside the node arrays: kv_upsert on add/update, kv_remove on delete. The +-- map lives on the per-plugin data table (pd.kv) and is populated lazily on the +-- first lookup (which also teaches us pd.key_attr for that plugin). +local function kv_upsert(pd, consumer) + if not pd.kv or not pd.key_attr then + return + end + local nc = core.table.clone(consumer) + nc.auth_conf = secret.fetch_secrets(nc.auth_conf, false) + -- fail closed: skip unset credentials or unresolved secret refs so a client + -- can never authenticate with a literal "$ENV://..." reference string. + if secret.has_secret_ref(nc.auth_conf) then + return + end + local key_value = nc.auth_conf[pd.key_attr] + if key_value == nil then + return + end + pd.kv[key_value] = nc + consumer._kvkey = key_value +end + +local function kv_remove(pd, consumer) + if pd.kv and consumer and consumer._kvkey ~= nil then + pd.kv[consumer._kvkey] = nil + end +end + +-- Collect current consumer/credential IDs from consumers.values. +local function collect_current_ids() + local ids = {} + for _, val in ipairs(consumers.values or {}) do + if type(val) == "table" and val.value and val.value.id then + ids[val.value.id] = true + end + end + return ids +end + +-- Remove all plugin node entries for a given consumer/credential ID. +-- Uses swap-with-last for O(1) array removal without holes. +local function remove_consumer_entries(eid) + local pset = entry_plugins[eid] + if not pset then return end + for pname in pairs(pset) do + local pd = cached_plugins[pname] + if pd then + local key = pname .. "\0" .. eid + local pos = node_index[key] + if pos and pos <= pd.len then + kv_remove(pd, pd.nodes[pos]) + if pos < pd.len then + -- swap with last element + local last = pd.nodes[pd.len] + pd.nodes[pos] = last + node_index[pname .. "\0" .. last._eid] = pos + end + pd.nodes[pd.len] = nil + pd.len = pd.len - 1 + end + node_index[key] = nil + end + end + entry_plugins[eid] = nil +end + +-- Add a consumer/credential to the appropriate auth plugin nodes. +local function add_consumer_entry(val) + if type(val) ~= "table" or not val.value then return end + local eid = val.value.id + if not eid then return end + for name, config in pairs(val.value.plugins or {}) do + local plugin_obj = plugin.get(name) + if not plugin_obj or plugin_obj.type ~= "auth" then + goto next_plugin + end + if not cached_plugins[name] then + cached_plugins[name] = { + nodes = {}, len = 0, + conf_version = consumers.conf_version + } + end + local consumer, err = construct_consumer_data(val, name, config) + if not consumer then + core.log.error("incremental: failed to construct consumer for plugin ", + name, ": ", err) + goto next_plugin + end + consumer._eid = eid + local pd = cached_plugins[name] + pd.len = pd.len + 1 + pd.nodes[pd.len] = consumer + node_index[name .. "\0" .. eid] = pd.len + if not entry_plugins[eid] then entry_plugins[eid] = {} end + entry_plugins[eid][name] = true + kv_upsert(pd, consumer) + ::next_plugin:: + end +end + +-- Full rebuild: construct entire tree and build indexes from scratch. +local function full_rebuild() + cached_plugins = plugin_consumer() + node_index = {} + entry_plugins = {} + for pname, pd in pairs(cached_plugins) do + for i = 1, pd.len do + local c = pd.nodes[i] + local eid = c.credential_id or c.consumer_name + c._eid = eid + node_index[pname .. "\0" .. eid] = i + if not entry_plugins[eid] then entry_plugins[eid] = {} end + entry_plugins[eid][pname] = true + end + end + tracked_count = consumers.values and #consumers.values or 0 + cached_conf_version = consumers.conf_version + core.table.clear(pending_set) + has_pending = false + pending_delete = false + core.log.info("consumer plugin tree fully rebuilt, conf_version: ", + cached_conf_version, ", tracked: ", tracked_count) +end + +-- Incremental update: process only pending changes, then verify for deletes. +local function apply_incremental() + -- Process pending upserts from filter callback + for eid, val in pairs(pending_set) do + remove_consumer_entries(eid) + add_consumer_entry(val) + end + core.table.clear(pending_set) + + -- Deletes arrive as value-less events; the watch does not hand us an id we + -- can map to an auth entry (consumer vs credential), so when a delete was + -- flagged we reconcile the incremental index against the current id set. + -- This is an O(N) id scan with NO consumer-data construction (the expensive + -- part), so it stays cheap even with a large consumer base, and it runs only + -- when a delete actually happened -- independent of whether the net count + -- went down (e.g. creates and deletes interleaved under churn). + if pending_delete then + local cur_ids = collect_current_ids() + -- Collect first, then remove (can't modify entry_plugins during pairs iteration) + local to_remove = {} + for eid in pairs(entry_plugins) do + if not cur_ids[eid] then + to_remove[#to_remove + 1] = eid + end + end + for _, eid in ipairs(to_remove) do + remove_consumer_entries(eid) + end + pending_delete = false + end + + has_pending = false + tracked_count = consumers.values and #consumers.values or 0 + + -- Update conf_version on all plugin data; keep the (incrementally maintained) + -- key_value map version in lockstep so _M.consumers_kv() serves it without a + -- full rebuild. + for _, pd in pairs(cached_plugins) do + pd.conf_version = consumers.conf_version + if pd.kv then + pd.kv_version = consumers.conf_version + end + end + cached_conf_version = consumers.conf_version +end + + _M.filter_consumers_list = filter_consumers_list function _M.get_consumer_key_from_credential_key(key) @@ -198,9 +393,12 @@ function _M.get_consumer_key_from_credential_key(key) end function _M.plugin(plugin_name) - local plugin_conf = core.lrucache.global("/consumers", - consumers.conf_version, plugin_consumer) - return plugin_conf[plugin_name] + if not cached_plugins then + full_rebuild() + elseif cached_conf_version ~= consumers.conf_version then + apply_incremental() + end + return cached_plugins[plugin_name] end function _M.consumers_conf(plugin_name) @@ -262,6 +460,11 @@ function create_consume_cache(consumers_conf, key_attr) else consumer_names[key_value] = new_consumer + -- Record the key on the node so an incremental delete/update can + -- remove this entry from the map (kv_remove). Nodes added later go + -- through kv_upsert which sets this too; setting it here covers the + -- nodes that were in the initial/full build. + consumer._kvkey = key_value end end @@ -272,10 +475,19 @@ end function _M.consumers_kv(plugin_name, consumer_conf, key_attr) - local consumers = lrucache("consumers_key#" .. plugin_name, consumer_conf.conf_version, - create_consume_cache, consumer_conf, key_attr) + -- consumer_conf is the per-plugin node set from _M.plugin(); the key_value -> + -- consumer map is cached on it and maintained incrementally by kv_upsert/ + -- kv_remove, so it is not rebuilt on every conf_version change. Rebuild only on + -- first use, a key_attr change, or a version gap the incremental path missed. + if consumer_conf.kv and consumer_conf.key_attr == key_attr + and consumer_conf.kv_version == consumer_conf.conf_version then + return consumer_conf.kv + end - return consumers + consumer_conf.kv = create_consume_cache(consumer_conf, key_attr) + consumer_conf.key_attr = key_attr + consumer_conf.kv_version = consumer_conf.conf_version + return consumer_conf.kv end @@ -309,10 +521,63 @@ end local function filter(consumer) - if not consumer.value or not consumer.value.plugins then + -- A delete arrives as a value-less event (the etcd watch sets value = nil on + -- removal). Flag it so the next incremental apply reconciles removed ids and + -- the deleted consumer stops authenticating on the next request. + if not consumer.value then + if cached_plugins then + pending_delete = true + has_pending = true + end + return + end + + if not consumer.value.plugins then return end plugin.set_plugins_meta_parent(consumer.value.plugins, consumer) + + -- Track changed consumer for incremental rebuild + if cached_plugins and consumer.value.id then + pending_set[consumer.value.id] = consumer + has_pending = true + end +end + + +-- Background consistency check: lightweight scan to detect stale entries. +-- Only triggers a full rebuild if an actual discrepancy is found. +-- This avoids the O(N) construction cost of plugin_consumer() on every tick. +local function background_full_rebuild(premature) + if premature then return end + if not consumers or not consumers.values then return end + if not cached_plugins then return end + + -- Process any pending changes first + if has_pending then + apply_incremental() + end + + -- Quick count check + local cur_count = #consumers.values + if cur_count ~= tracked_count then + core.log.info("background: count mismatch (tracked=", tracked_count, + ", actual=", cur_count, "), full rebuild") + full_rebuild() + return + end + + -- Deep consistency check: look for stale entries in our index. + -- This catches the rare simultaneous create+delete case where + -- count stays the same but different consumers are present. + local cur_ids = collect_current_ids() + for eid in pairs(entry_plugins) do + if not cur_ids[eid] then + core.log.warn("background: stale entry ", eid, " detected, full rebuild") + full_rebuild() + return + end + end end @@ -329,6 +594,11 @@ function _M.init_worker() error("failed to create etcd instance for fetching consumers: " .. err) return end + + local ok, timer_err = ngx.timer.every(FULL_SYNC_INTERVAL, background_full_rebuild) + if not ok then + core.log.error("failed to create consumer full rebuild timer: ", timer_err) + end end local function get_anonymous_consumer_from_local_cache(name) diff --git a/t/node/consumer-incremental-rebuild.t b/t/node/consumer-incremental-rebuild.t new file mode 100644 index 000000000000..8931a713d407 --- /dev/null +++ b/t/node/consumer-incremental-rebuild.t @@ -0,0 +1,302 @@ +# +# 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(); +log_level("info"); +run_tests; + +__DATA__ + +=== TEST 1: key-auth route +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + 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: create consumers alice and ghost (both present before the first request) +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + t('/apisix/admin/consumers', ngx.HTTP_PUT, + [[{ "username": "alice", "plugins": { "key-auth": { "key": "alice-key" } } }]]) + -- ghost is created here (before TEST 3's first auth request) and never + -- updated, so its key_value map entry is produced by the initial full + -- build, not by an incremental upsert. It is deleted in TEST 17 to prove + -- the full-build path is also removable. + local code = t('/apisix/admin/consumers', ngx.HTTP_PUT, + [[{ "username": "ghost", "plugins": { "key-auth": { "key": "ghost-key" } } }]]) + if code >= 300 then ngx.status = code end + ngx.say("passed") + } + } +--- response_body +passed + + + +=== TEST 3: bootstrap + create takes effect on next request (alice authenticates) +--- request +GET /hello +--- more_headers +apikey: alice-key +--- response_body +hello world +--- error_log +consumer plugin tree fully rebuilt + + + +=== TEST 4: create a second consumer bob (incremental add, no full rebuild) +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/consumers', + ngx.HTTP_PUT, + [[{ + "username": "bob", + "plugins": { "key-auth": { "key": "bob-key" } } + }]] + ) + if code >= 300 then ngx.status = code end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 5: bob authenticates and alice still works +--- pipelined_requests eval +["GET /hello", "GET /hello"] +--- more_headers eval +["apikey: bob-key", "apikey: alice-key"] +--- response_body eval +["hello world\n", "hello world\n"] + + + +=== TEST 6: update alice's key +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/consumers', + ngx.HTTP_PUT, + [[{ + "username": "alice", + "plugins": { "key-auth": { "key": "alice-key-v2" } } + }]] + ) + if code >= 300 then ngx.status = code end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 7: old key rejected after update +--- request +GET /hello +--- more_headers +apikey: alice-key +--- error_code: 401 + + + +=== TEST 8: new key accepted after update +--- request +GET /hello +--- more_headers +apikey: alice-key-v2 +--- response_body +hello world + + + +=== TEST 9: delete bob +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/consumers/bob', ngx.HTTP_DELETE) + if code >= 300 then ngx.status = code end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 10: deleted consumer's key is rejected +--- request +GET /hello +--- more_headers +apikey: bob-key +--- error_code: 401 + + + +=== TEST 11: consumer with a credential + consumer group plugins still merge +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + -- consumer group carries a response-rewrite plugin + local code = t('/apisix/admin/consumer_groups/cg1', ngx.HTTP_PUT, + [[{ + "plugins": { + "response-rewrite": { "headers": { "set": { "X-Group": "cg1" } } } + } + }]]) + if code >= 300 then ngx.status = code; ngx.say("group"); return end + + -- consumer with NO direct plugins, in the group; key-auth lives on a credential + code = t('/apisix/admin/consumers', ngx.HTTP_PUT, + [[{ "username": "carol", "group_id": "cg1" }]]) + if code >= 300 then ngx.status = code; ngx.say("consumer"); return end + + code = t('/apisix/admin/consumers/carol/credentials/cred-1', ngx.HTTP_PUT, + [[{ "plugins": { "key-auth": { "key": "carol-key" } } }]]) + if code >= 300 then ngx.status = code; ngx.say("credential"); return end + + ngx.say("passed") + } + } +--- response_body +passed + + + +=== TEST 12: credential authenticates and the consumer group plugin is applied +--- request +GET /hello +--- more_headers +apikey: carol-key +--- response_body +hello world +--- response_headers +X-Group: cg1 + + + +=== TEST 13: create a consumer that we will delete under concurrent creates +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code = t('/apisix/admin/consumers', ngx.HTTP_PUT, + [[{ "username": "victim", "plugins": { "key-auth": { "key": "victim-key" } } }]]) + if code >= 300 then ngx.status = code end + ngx.say("passed") + } + } +--- response_body +passed + + + +=== TEST 14: victim authenticates +--- request +GET /hello +--- more_headers +apikey: victim-key +--- response_body +hello world + + + +=== TEST 15: delete victim WHILE creating two consumers (net consumer count RISES) +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + -- two creates + one delete in the same sync window: the total count + -- goes up, so a delete detector keyed on "count decreased" would miss + -- this. The delete must still take effect. + t('/apisix/admin/consumers', ngx.HTTP_PUT, + [[{ "username": "filler_a", "plugins": { "key-auth": { "key": "filler-a-key" } } }]]) + t('/apisix/admin/consumers', ngx.HTTP_PUT, + [[{ "username": "filler_b", "plugins": { "key-auth": { "key": "filler-b-key" } } }]]) + local code = t('/apisix/admin/consumers/victim', ngx.HTTP_DELETE) + if code >= 300 then ngx.status = code end + ngx.say("passed") + } + } +--- response_body +passed + + + +=== TEST 16: victim key rejected immediately despite net-positive churn; fillers work +--- pipelined_requests eval +["GET /hello", "GET /hello", "GET /hello"] +--- more_headers eval +["apikey: victim-key", "apikey: filler-a-key", "apikey: filler-b-key"] +--- error_code eval +[401, 200, 200] + + + +=== TEST 17: delete ghost (a consumer built by the initial full build, never upserted) +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code = t('/apisix/admin/consumers/ghost', ngx.HTTP_DELETE) + if code >= 300 then ngx.status = code end + ngx.say("passed") + } + } +--- response_body +passed + + + +=== TEST 18: ghost's key is rejected (full-build entry was removed from the kv map) +--- request +GET /hello +--- more_headers +apikey: ghost-key +--- error_code: 401 From d9317baa660a7d9fd88a4f9747f6219e05189a04 Mon Sep 17 00:00:00 2001 From: Jairo Laupa Date: Tue, 21 Jul 2026 10:14:53 +0200 Subject: [PATCH 2/3] address review: normalize credential ids, propagate parent updates, revoke on empty plugins, TTL-bound secret refresh Follow-up to @juzhiyuan's review: - Normalize entity ids across both paths: the full build now keys nodes by the same "/consumers"-relative id the incremental events use (stamped in plugin_consumer), so bootstrapped credentials can be updated/removed and the consistency timer no longer flags them as stale. Fixes cross-consumer collisions between credentials that share a leaf id. - Enqueue an entity on change even when its new plugin set is empty, so removing all of a consumer's auth plugins removes its old auth entries. - On a parent consumer change, refresh all of its child credential entries (they clone the parent's group_id/labels/custom_id), so credential-auth'd requests do not keep stale group policy/headers. - Give the per-plugin key map a TTL (KV_TTL, matching the previous global lru-cache 300s bound) so secret-backed auth values are re-derived even though /secrets changes do not bump conf_version. - Remove the now-unused module-level lrucache (lint). Tests: add remove-all-plugins, duplicate credential leaf ids across consumers, credential rotation, and parent-group-change-propagates-to-credential cases. --- apisix/consumer.lua | 86 +++++++++++++-- t/node/consumer-incremental-rebuild.t | 150 ++++++++++++++++++++++++-- 2 files changed, 218 insertions(+), 18 deletions(-) diff --git a/apisix/consumer.lua b/apisix/consumer.lua index 6e980d93f333..de89b174189c 100644 --- a/apisix/consumer.lua +++ b/apisix/consumer.lua @@ -32,10 +32,6 @@ local _M = { version = 0.3, } -local lrucache = core.lrucache.new({ - ttl = 300, count = 512 -}) - -- Please calculate and set the value of the "consumers_count_for_lrucache" -- variable based on the number of consumers in the current environment, -- taking into account the appropriate adjustment coefficient. @@ -176,6 +172,11 @@ function plugin_consumer() goto CONTINUE end + -- Stamp the canonical entity id (relative to /consumers, e.g. + -- "jack" or "jack/credentials/cred-1") so the incremental index + -- and the full-build index agree on ids for consumers AND + -- credentials (avoids cross-consumer credential-id collisions). + consumer._eid = val.value.id plugins[name].len = plugins[name].len + 1 core.table.insert(plugins[name].nodes, plugins[name].len, consumer) @@ -207,6 +208,23 @@ local entry_plugins = {} -- eid -> { [plugin_name] = true } local pending_set = {} -- eid -> consumer item (from filter callback) local has_pending = false local pending_delete = false -- a delete event was seen; reconcile removed ids +local cred_by_consumer = {} -- consumer_name -> { credential_entity_id -> true } +local KV_TTL = 300 -- seconds; refresh bound for secret-backed auth values + +-- Entity ids are the key relative to /consumers: "jack" for a consumer, +-- "jack/credentials/cred-1" for a credential. These helpers let the parent +-- consumer refresh its child credential entries on update. +local function is_credential_id(eid) + return type(eid) == "string" and eid:find("/credentials/", 1, true) ~= nil +end + +local function parent_consumer_name(eid) + local i = eid:find("/credentials/", 1, true) + if i then + return eid:sub(1, i - 1) + end + return eid +end local tracked_count = 0 -- count of consumers.values at last sync local FULL_SYNC_INTERVAL = 30 -- seconds between background consistency checks @@ -278,6 +296,14 @@ local function remove_consumer_entries(eid) end end entry_plugins[eid] = nil + if is_credential_id(eid) then + local pc = parent_consumer_name(eid) + local set = cred_by_consumer[pc] + if set then + set[eid] = nil + if next(set) == nil then cred_by_consumer[pc] = nil end + end + end end -- Add a consumer/credential to the appropriate auth plugin nodes. @@ -312,6 +338,11 @@ local function add_consumer_entry(val) kv_upsert(pd, consumer) ::next_plugin:: end + if is_credential_id(eid) then + local pc = parent_consumer_name(eid) + cred_by_consumer[pc] = cred_by_consumer[pc] or {} + cred_by_consumer[pc][eid] = true + end end -- Full rebuild: construct entire tree and build indexes from scratch. @@ -319,14 +350,19 @@ local function full_rebuild() cached_plugins = plugin_consumer() node_index = {} entry_plugins = {} + core.table.clear(cred_by_consumer) for pname, pd in pairs(cached_plugins) do for i = 1, pd.len do local c = pd.nodes[i] - local eid = c.credential_id or c.consumer_name - c._eid = eid + local eid = c._eid -- canonical id stamped by plugin_consumer() node_index[pname .. "\0" .. eid] = i if not entry_plugins[eid] then entry_plugins[eid] = {} end entry_plugins[eid][pname] = true + if is_credential_id(eid) then + local pc = parent_consumer_name(eid) + cred_by_consumer[pc] = cred_by_consumer[pc] or {} + cred_by_consumer[pc][eid] = true + end end end tracked_count = consumers.values and #consumers.values or 0 @@ -344,6 +380,25 @@ local function apply_incremental() for eid, val in pairs(pending_set) do remove_consumer_entries(eid) add_consumer_entry(val) + -- Credentials clone their parent consumer (group_id, labels, custom_id), + -- so a parent change must refresh every child credential entry too, or + -- credential-authenticated requests keep stale group policy/headers. + if not is_credential_id(eid) then + local creds = cred_by_consumer[eid] + if creds then + local list = {} + for cred_eid in pairs(creds) do + list[#list + 1] = cred_eid + end + for _, cred_eid in ipairs(list) do + local citem = consumers:get(cred_eid) + if citem then + remove_consumer_entries(cred_eid) + add_consumer_entry(citem) + end + end + end + end end core.table.clear(pending_set) @@ -479,14 +534,22 @@ function _M.consumers_kv(plugin_name, consumer_conf, key_attr) -- consumer map is cached on it and maintained incrementally by kv_upsert/ -- kv_remove, so it is not rebuilt on every conf_version change. Rebuild only on -- first use, a key_attr change, or a version gap the incremental path missed. + -- Rebuild on first use, a key_attr change, a version gap the incremental path + -- did not cover, or once the TTL lapses. The TTL preserves the refresh bound + -- the previous global lru-cache provided: /secrets or external secret changes + -- do not bump conf_version, so secret-backed auth values are re-derived at + -- least every KV_TTL seconds. + local now = ngx.now() if consumer_conf.kv and consumer_conf.key_attr == key_attr - and consumer_conf.kv_version == consumer_conf.conf_version then + and consumer_conf.kv_version == consumer_conf.conf_version + and (now - (consumer_conf.kv_built_at or 0)) < KV_TTL then return consumer_conf.kv end consumer_conf.kv = create_consume_cache(consumer_conf, key_attr) consumer_conf.key_attr = key_attr consumer_conf.kv_version = consumer_conf.conf_version + consumer_conf.kv_built_at = now return consumer_conf.kv end @@ -532,12 +595,13 @@ local function filter(consumer) return end - if not consumer.value.plugins then - return + if consumer.value.plugins then + plugin.set_plugins_meta_parent(consumer.value.plugins, consumer) end - plugin.set_plugins_meta_parent(consumer.value.plugins, consumer) - -- Track changed consumer for incremental rebuild + -- Enqueue the entity even when it now has no plugins, so removing all of a + -- consumer's auth plugins removes its old auth entries instead of leaving + -- the previous key valid. if cached_plugins and consumer.value.id then pending_set[consumer.value.id] = consumer has_pending = true diff --git a/t/node/consumer-incremental-rebuild.t b/t/node/consumer-incremental-rebuild.t index 8931a713d407..741fef3a0011 100644 --- a/t/node/consumer-incremental-rebuild.t +++ b/t/node/consumer-incremental-rebuild.t @@ -56,10 +56,7 @@ passed local t = require("lib.test_admin").test t('/apisix/admin/consumers', ngx.HTTP_PUT, [[{ "username": "alice", "plugins": { "key-auth": { "key": "alice-key" } } }]]) - -- ghost is created here (before TEST 3's first auth request) and never - -- updated, so its key_value map entry is produced by the initial full - -- build, not by an incremental upsert. It is deleted in TEST 17 to prove - -- the full-build path is also removable. + -- ghost exists before the first request (full-build path); deleted in TEST 17 local code = t('/apisix/admin/consumers', ngx.HTTP_PUT, [[{ "username": "ghost", "plugins": { "key-auth": { "key": "ghost-key" } } }]]) if code >= 300 then ngx.status = code end @@ -252,9 +249,8 @@ hello world location /t { content_by_lua_block { local t = require("lib.test_admin").test - -- two creates + one delete in the same sync window: the total count - -- goes up, so a delete detector keyed on "count decreased" would miss - -- this. The delete must still take effect. + -- two creates + one delete: net count rises, so a count-decrease + -- delete detector would miss it; the delete must still take effect t('/apisix/admin/consumers', ngx.HTTP_PUT, [[{ "username": "filler_a", "plugins": { "key-auth": { "key": "filler-a-key" } } }]]) t('/apisix/admin/consumers', ngx.HTTP_PUT, @@ -300,3 +296,143 @@ GET /hello --- more_headers apikey: ghost-key --- error_code: 401 + + + +=== TEST 19: consumer with all auth plugins removed +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + t('/apisix/admin/consumers', ngx.HTTP_PUT, + [[{ "username": "zoe", "plugins": { "key-auth": { "key": "zoe-key" } } }]]) + -- remove every auth plugin: the old key must stop working + local code = t('/apisix/admin/consumers', ngx.HTTP_PUT, + [[{ "username": "zoe", "plugins": {} }]]) + if code >= 300 then ngx.status = code end + ngx.say("passed") + } + } +--- response_body +passed + + + +=== TEST 20: key of a consumer whose auth plugins were all removed is rejected +--- request +GET /hello +--- more_headers +apikey: zoe-key +--- error_code: 401 + + + +=== TEST 21: two consumers sharing the same credential leaf id "cred-1" +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + t('/apisix/admin/consumers', ngx.HTTP_PUT, [[{ "username": "jack" }]]) + t('/apisix/admin/consumers/jack/credentials/cred-1', ngx.HTTP_PUT, + [[{ "plugins": { "key-auth": { "key": "jack-cred1" } } }]]) + t('/apisix/admin/consumers', ngx.HTTP_PUT, [[{ "username": "mary" }]]) + local code = t('/apisix/admin/consumers/mary/credentials/cred-1', ngx.HTTP_PUT, + [[{ "plugins": { "key-auth": { "key": "mary-cred1" } } }]]) + if code >= 300 then ngx.status = code end + ngx.say("passed") + } + } +--- response_body +passed + + + +=== TEST 22: both duplicate-leaf-id credentials authenticate independently +--- pipelined_requests eval +["GET /hello", "GET /hello"] +--- more_headers eval +["apikey: jack-cred1", "apikey: mary-cred1"] +--- response_body eval +["hello world\n", "hello world\n"] + + + +=== TEST 23: delete jack's credential; mary's same-leaf-id credential is unaffected +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code = t('/apisix/admin/consumers/jack/credentials/cred-1', ngx.HTTP_DELETE) + if code >= 300 then ngx.status = code end + ngx.say("passed") + } + } +--- response_body +passed + + + +=== TEST 24: jack's credential is rejected, mary's still works (no cross-consumer collision) +--- pipelined_requests eval +["GET /hello", "GET /hello"] +--- more_headers eval +["apikey: jack-cred1", "apikey: mary-cred1"] +--- error_code eval +[401, 200] + + + +=== TEST 25: parent consumer group change propagates to credential-authenticated requests +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + t('/apisix/admin/consumer_groups/cg-a', ngx.HTTP_PUT, + [[{ "plugins": { "response-rewrite": { "headers": { "set": { "X-Grp": "A" } } } } }]]) + t('/apisix/admin/consumer_groups/cg-b', ngx.HTTP_PUT, + [[{ "plugins": { "response-rewrite": { "headers": { "set": { "X-Grp": "B" } } } } }]]) + t('/apisix/admin/consumers', ngx.HTTP_PUT, [[{ "username": "pat", "group_id": "cg-a" }]]) + local code = t('/apisix/admin/consumers/pat/credentials/c1', ngx.HTTP_PUT, + [[{ "plugins": { "key-auth": { "key": "pat-key" } } }]]) + if code >= 300 then ngx.status = code end + ngx.say("passed") + } + } +--- response_body +passed + + + +=== TEST 26: credential request sees parent's original group +--- request +GET /hello +--- more_headers +apikey: pat-key +--- response_headers +X-Grp: A + + + +=== TEST 27: move the parent to a different group; credential request must reflect it +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code = t('/apisix/admin/consumers', ngx.HTTP_PUT, + [[{ "username": "pat", "group_id": "cg-b" }]]) + if code >= 300 then ngx.status = code end + ngx.say("passed") + } + } +--- response_body +passed + + + +=== TEST 28: credential-authenticated request now reflects the parent's new group +--- request +GET /hello +--- more_headers +apikey: pat-key +--- response_headers +X-Grp: B From b6ef18c7d85f704be7c4ab7599eb7ecbd6f5f83c Mon Sep 17 00:00:00 2001 From: Jairo Laupa Date: Tue, 21 Jul 2026 13:19:22 +0200 Subject: [PATCH 3/3] test(consumer): add missing request section to config setup blocks The admin-setup blocks used a --- config/location /t stanza with no --- request line, so Test::Nginx had no request to send and aborted the whole file at the first such block (Request line should be non-empty). Add --- request / GET /t to each config-only block so the setup requests actually fire and the suite runs. --- t/node/consumer-incremental-rebuild.t | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/t/node/consumer-incremental-rebuild.t b/t/node/consumer-incremental-rebuild.t index 741fef3a0011..a22623e1d1b1 100644 --- a/t/node/consumer-incremental-rebuild.t +++ b/t/node/consumer-incremental-rebuild.t @@ -44,6 +44,8 @@ __DATA__ ngx.say(body) } } +--- request +GET /t --- response_body passed @@ -63,6 +65,8 @@ passed ngx.say("passed") } } +--- request +GET /t --- response_body passed @@ -96,6 +100,8 @@ consumer plugin tree fully rebuilt ngx.say(body) } } +--- request +GET /t --- response_body passed @@ -127,6 +133,8 @@ passed ngx.say(body) } } +--- request +GET /t --- response_body passed @@ -161,6 +169,8 @@ hello world ngx.say(body) } } +--- request +GET /t --- response_body passed @@ -201,6 +211,8 @@ apikey: bob-key ngx.say("passed") } } +--- request +GET /t --- response_body passed @@ -229,6 +241,8 @@ X-Group: cg1 ngx.say("passed") } } +--- request +GET /t --- response_body passed @@ -260,6 +274,8 @@ hello world ngx.say("passed") } } +--- request +GET /t --- response_body passed @@ -285,6 +301,8 @@ passed ngx.say("passed") } } +--- request +GET /t --- response_body passed @@ -313,6 +331,8 @@ apikey: ghost-key ngx.say("passed") } } +--- request +GET /t --- response_body passed @@ -342,6 +362,8 @@ apikey: zoe-key ngx.say("passed") } } +--- request +GET /t --- response_body passed @@ -367,6 +389,8 @@ passed ngx.say("passed") } } +--- request +GET /t --- response_body passed @@ -398,6 +422,8 @@ passed ngx.say("passed") } } +--- request +GET /t --- response_body passed @@ -424,6 +450,8 @@ X-Grp: A ngx.say("passed") } } +--- request +GET /t --- response_body passed