Skip to content

Commit afe61d7

Browse files
janiussyafiqjaniussyafiq
authored
fix(plugin): merge consumer group plugins when consumer has no direct plugins (apache#12998)
1 parent 0c6ca07 commit afe61d7

File tree

2 files changed

+261
-12
lines changed

2 files changed

+261
-12
lines changed

‎apisix/plugin.lua‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -706,16 +706,21 @@ end
706706

707707

708708
local function merge_consumer_route(route_conf, consumer_conf, consumer_group_conf)
709-
if not consumer_conf.plugins or
710-
core.table.nkeys(consumer_conf.plugins) == 0
711-
then
712-
core.log.info("consumer no plugins")
709+
local has_consumer_plugins = consumer_conf.plugins and
710+
core.table.nkeys(consumer_conf.plugins) > 0
711+
local has_group_plugins = consumer_group_conf and
712+
consumer_group_conf.value and
713+
consumer_group_conf.value.plugins and
714+
core.table.nkeys(consumer_group_conf.value.plugins) > 0
715+
716+
if not has_consumer_plugins and not has_group_plugins then
717+
core.log.info("consumer and consumer group have no plugins")
713718
return route_conf
714719
end
715720

716721
local new_route_conf = core.table.deepcopy(route_conf)
717722

718-
if consumer_group_conf then
723+
if has_group_plugins then
719724
for name, conf in pairs(consumer_group_conf.value.plugins) do
720725
if not new_route_conf.value.plugins then
721726
new_route_conf.value.plugins = {}
@@ -729,15 +734,17 @@ local function merge_consumer_route(route_conf, consumer_conf, consumer_group_co
729734
end
730735

731736

732-
for name, conf in pairs(consumer_conf.plugins) do
733-
if not new_route_conf.value.plugins then
734-
new_route_conf.value.plugins = {}
735-
end
737+
if has_consumer_plugins then
738+
for name, conf in pairs(consumer_conf.plugins) do
739+
if not new_route_conf.value.plugins then
740+
new_route_conf.value.plugins = {}
741+
end
736742

737-
if new_route_conf.value.plugins[name] == nil then
738-
conf._from_consumer = true
743+
if new_route_conf.value.plugins[name] == nil then
744+
conf._from_consumer = true
745+
end
746+
new_route_conf.value.plugins[name] = conf
739747
end
740-
new_route_conf.value.plugins[name] = conf
741748
end
742749

743750
return new_route_conf

‎t/plugin/limit-count-consumer-group-credentials.t‎

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
#
2+
# Licensed to the Apache Software Foundation (ASF) under one or more
3+
# contributor license agreements. See the NOTICE file distributed with
4+
# this work for additional information regarding copyright ownership.
5+
# The ASF licenses this file to You under the Apache License, Version 2.0
6+
# (the "License"); you may not use this file except in compliance with
7+
# the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
#
17+
use t::APISIX 'no_plan';
18+
19+
repeat_each(1);
20+
no_long_string();
21+
no_root_location();
22+
23+
add_block_preprocessor(sub {
24+
my ($block) = @_;
25+
if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
26+
$block->set_value("no_error_log", "[error]");
27+
}
28+
if (!defined $block->request) {
29+
$block->set_value("request", "GET /t");
30+
}
31+
});
32+
33+
run_tests();
34+
35+
__DATA__
36+
37+
=== TEST 1: setup consumer group with limit-count plugin
38+
--- config
39+
location /t {
40+
content_by_lua_block {
41+
local t = require("lib.test_admin").test
42+
local code, body = t('/apisix/admin/consumer_groups/test_group',
43+
ngx.HTTP_PUT,
44+
[[{
45+
"plugins": {
46+
"limit-count": {
47+
"count": 2,
48+
"time_window": 60,
49+
"rejected_code": 429,
50+
"key": "remote_addr",
51+
"policy": "local"
52+
}
53+
}
54+
}]]
55+
)
56+
57+
if code >= 300 then
58+
ngx.status = code
59+
end
60+
ngx.say(body)
61+
}
62+
}
63+
--- response_body
64+
passed
65+
66+
67+
68+
=== TEST 2: setup consumer with group_id (no direct plugins)
69+
--- config
70+
location /t {
71+
content_by_lua_block {
72+
local t = require("lib.test_admin").test
73+
local code, body = t('/apisix/admin/consumers/jack',
74+
ngx.HTTP_PUT,
75+
[[{
76+
"username": "jack",
77+
"group_id": "test_group"
78+
}]]
79+
)
80+
81+
if code >= 300 then
82+
ngx.status = code
83+
end
84+
ngx.say(body)
85+
}
86+
}
87+
--- response_body
88+
passed
89+
90+
91+
92+
=== TEST 3: setup credentials via credentials endpoint
93+
--- config
94+
location /t {
95+
content_by_lua_block {
96+
local t = require("lib.test_admin").test
97+
local code, body = t('/apisix/admin/consumers/jack/credentials/cred-jack',
98+
ngx.HTTP_PUT,
99+
[[{
100+
"id": "cred-jack",
101+
"plugins": {
102+
"key-auth": {
103+
"key": "auth-jack"
104+
}
105+
}
106+
}]]
107+
)
108+
109+
if code >= 300 then
110+
ngx.status = code
111+
end
112+
ngx.say(body)
113+
}
114+
}
115+
--- response_body
116+
passed
117+
118+
119+
120+
=== TEST 4: setup route with key-auth
121+
--- config
122+
location /t {
123+
content_by_lua_block {
124+
local t = require("lib.test_admin").test
125+
local code, body = t('/apisix/admin/routes/1',
126+
ngx.HTTP_PUT,
127+
[[{
128+
"plugins": {
129+
"key-auth": {}
130+
},
131+
"upstream": {
132+
"nodes": {
133+
"127.0.0.1:1980": 1
134+
},
135+
"type": "roundrobin"
136+
},
137+
"uri": "/hello"
138+
}]]
139+
)
140+
141+
if code >= 300 then
142+
ngx.status = code
143+
end
144+
ngx.say(body)
145+
}
146+
}
147+
--- response_body
148+
passed
149+
150+
151+
152+
=== TEST 5: verify rate limiting works (all 3 requests in one test)
153+
--- pipelined_requests eval
154+
[
155+
"GET /hello", "GET /hello", "GET /hello"
156+
]
157+
--- more_headers
158+
apikey: auth-jack
159+
--- error_code eval
160+
[200, 200, 429]
161+
162+
163+
164+
=== TEST 6: setup second consumer group with higher limit
165+
--- config
166+
location /t {
167+
content_by_lua_block {
168+
local t = require("lib.test_admin").test
169+
local code, body = t('/apisix/admin/consumer_groups/premium_group',
170+
ngx.HTTP_PUT,
171+
[[{
172+
"plugins": {
173+
"limit-count": {
174+
"count": 10,
175+
"time_window": 60,
176+
"rejected_code": 429,
177+
"key": "remote_addr",
178+
"policy": "local"
179+
}
180+
}
181+
}]]
182+
)
183+
184+
ngx.say(body)
185+
}
186+
}
187+
--- response_body
188+
passed
189+
190+
191+
192+
=== TEST 7: setup premium consumer with credentials
193+
--- config
194+
location /t {
195+
content_by_lua_block {
196+
local t = require("lib.test_admin").test
197+
198+
local code, body = t('/apisix/admin/consumers/jane',
199+
ngx.HTTP_PUT,
200+
[[{
201+
"username": "jane",
202+
"group_id": "premium_group"
203+
}]]
204+
)
205+
206+
if code >= 300 then
207+
ngx.status = code
208+
ngx.say(body)
209+
return
210+
end
211+
212+
code, body = t('/apisix/admin/consumers/jane/credentials/cred-jane',
213+
ngx.HTTP_PUT,
214+
[[{
215+
"id": "cred-jane",
216+
"plugins": {
217+
"key-auth": {
218+
"key": "auth-jane"
219+
}
220+
}
221+
}]]
222+
)
223+
224+
ngx.say(body)
225+
}
226+
}
227+
--- response_body
228+
passed
229+
230+
231+
232+
=== TEST 8: verify premium consumer has higher limit
233+
--- pipelined_requests eval
234+
[
235+
"GET /hello", "GET /hello", "GET /hello", "GET /hello", "GET /hello",
236+
"GET /hello", "GET /hello", "GET /hello", "GET /hello", "GET /hello",
237+
"GET /hello"
238+
]
239+
--- more_headers
240+
apikey: auth-jane
241+
--- error_code eval
242+
[200, 200, 200, 200, 200, 200, 200, 200, 200, 200, 429]

0 commit comments

Comments
 (0)
⚡