Skip to content

Commit 9d2d9cf

Browse files
AlinsRanAlinsRan
authored
fix(standalone): API-driven mode does not properly handle consumer schema (#12256)
1 parent 9112923 commit 9d2d9cf

File tree

7 files changed

+192
-49
lines changed

7 files changed

+192
-49
lines changed

‎apisix/admin/standalone.lua‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,12 @@ local function update(ctx)
156156
end
157157
end
158158
if item_checker then
159-
valid, err = item_checker(item_temp)
159+
local item_checker_key
160+
if item.id then
161+
-- credential need to check key
162+
item_checker_key = "/" .. key .. "/" .. item_temp.id
163+
end
164+
valid, err = item_checker(item_temp, item_checker_key)
160165
if not valid then
161166
core.log.error(err_prefix, err)
162167
core.response.exit(400, {error_msg = err_prefix .. err})
@@ -235,6 +240,56 @@ function _M.run()
235240
end
236241

237242

243+
local patch_schema
244+
do
245+
local resource_schema = {
246+
"proto",
247+
"global_rule",
248+
"route",
249+
"service",
250+
"upstream",
251+
"consumer",
252+
"consumer_group",
253+
"credential",
254+
"ssl",
255+
"plugin_config",
256+
}
257+
local function attach_modifiedIndex_schema(name)
258+
local schema = core.schema[name]
259+
if not schema then
260+
core.log.error("schema for ", name, " not found")
261+
return
262+
end
263+
if schema.properties and not schema.properties.modifiedIndex then
264+
schema.properties.modifiedIndex = {
265+
type = "integer",
266+
}
267+
end
268+
end
269+
270+
local function patch_credential_schema()
271+
local credential_schema = core.schema["credential"]
272+
if credential_schema and credential_schema.properties then
273+
credential_schema.properties.id = {
274+
type = "string",
275+
minLength = 15,
276+
maxLength = 128,
277+
pattern = [[^[a-zA-Z0-9]+/credentials/[a-zA-Z0-9]+$]],
278+
}
279+
end
280+
end
281+
282+
function patch_schema()
283+
-- attach modifiedIndex schema to all resource schemas
284+
for _, name in ipairs(resource_schema) do
285+
attach_modifiedIndex_schema(name)
286+
end
287+
-- patch credential schema
288+
patch_credential_schema()
289+
end
290+
end
291+
292+
238293
function _M.init_worker()
239294
local function update_config()
240295
local config, err = shared_dict:get("config")
@@ -251,6 +306,8 @@ function _M.init_worker()
251306
config_yaml._update_config(config)
252307
end
253308
events:register(update_config, EVENT_UPDATE, EVENT_UPDATE)
309+
310+
patch_schema()
254311
end
255312

256313

‎apisix/consumer.lua‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@ local consumers_count_for_lrucache = 4096
4444
local function remove_etcd_prefix(key)
4545
local prefix = ""
4646
local local_conf = config_local.local_conf()
47-
if local_conf.etcd and local_conf.etcd.prefix then
47+
local role = core.table.try_read_attr(local_conf, "deployment", "role")
48+
local provider = core.table.try_read_attr(local_conf, "deployment", "role_" ..
49+
role, "config_provider")
50+
if provider == "etcd" and local_conf.etcd and local_conf.etcd.prefix then
4851
prefix = local_conf.etcd.prefix
4952
end
5053
return string_sub(key, #prefix + 1)
@@ -285,25 +288,12 @@ local function check_consumer(consumer, key)
285288
end
286289

287290

288-
local function filter(consumer)
289-
if not consumer.value then
290-
return
291-
end
292-
293-
-- We expect the id is the same as username. Fix up it here if it isn't.
294-
consumer.value.id = consumer.value.username
295-
end
296-
297-
298291
function _M.init_worker()
299292
local err
300293
local cfg = {
301294
automatic = true,
302295
checker = check_consumer,
303296
}
304-
if core.config.type ~= "etcd" then
305-
cfg.filter = filter
306-
end
307297

308298
consumers, err = core.config.new("/consumers", cfg)
309299
if not consumers then

‎apisix/core/config_etcd.lua‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,8 @@ local function load_full_data(self, dir_res, headers)
574574
end
575575

576576
if data_valid and self.checker then
577+
-- TODO: An opts table should be used
578+
-- as different checkers may use different parameters
577579
data_valid, err = self.checker(item.value, item.key)
578580
if not data_valid then
579581
log.error("failed to check item data of [", self.key,

‎apisix/core/config_yaml.lua‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ local yaml = require("lyaml")
2525
local log = require("apisix.core.log")
2626
local json = require("apisix.core.json")
2727
local new_tab = require("table.new")
28-
local tbl_deepcopy = require("apisix.core.table").deepcopy
2928
local check_schema = require("apisix.core.schema").check
3029
local profile = require("apisix.core.profile")
3130
local lfs = require("lfs")
@@ -239,7 +238,9 @@ local function sync_data(self)
239238
end
240239

241240
if data_valid and self.checker then
242-
data_valid, err = self.checker(item)
241+
-- TODO: An opts table should be used
242+
-- as different checkers may use different parameters
243+
data_valid, err = self.checker(item, conf_item.key)
243244
if not data_valid then
244245
log.error("failed to check item data of [", self.key,
245246
"] err:", err, " ,val: ", json.delay_encode(item))
@@ -274,7 +275,7 @@ local function sync_data(self)
274275
", it should be an object")
275276
end
276277

277-
local id = item.id or ("arr_" .. idx)
278+
local id = item.id or item.username or ("arr_" .. idx)
278279
local modifiedIndex = item.modifiedIndex or conf_version
279280
local conf_item = {value = item, modifiedIndex = modifiedIndex,
280281
key = "/" .. self.key .. "/" .. id}
@@ -288,7 +289,7 @@ local function sync_data(self)
288289
end
289290

290291
if data_valid and self.checker then
291-
data_valid, err = self.checker(item)
292+
data_valid, err = self.checker(item, conf_item.key)
292293
if not data_valid then
293294
log.error("failed to check item data of [", self.key,
294295
"] err:", err, " ,val: ", json.delay_encode(item))
@@ -447,16 +448,6 @@ function _M.new(key, opts)
447448
key = sub_str(key, 2)
448449
end
449450

450-
if is_use_admin_api() then
451-
if item_schema and item_schema.properties then
452-
local item_schema_cp = tbl_deepcopy(item_schema)
453-
-- allow clients to specify modifiedIndex to control resource changes.
454-
item_schema_cp.properties.modifiedIndex = {
455-
type = "integer",
456-
}
457-
item_schema = item_schema_cp
458-
end
459-
end
460451
local obj = setmetatable({
461452
automatic = automatic,
462453
item_schema = item_schema,

‎apisix/stream/router/ip_port.lua‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,9 @@ function _M.stream_init_worker(filter)
233233
user_routes, err = core.config.new("/stream_routes", {
234234
automatic = true,
235235
item_schema = core.schema.stream_route,
236-
checker = stream_route_checker,
236+
checker = function(item)
237+
return stream_route_checker(item)
238+
end,
237239
filter = filter,
238240
})
239241

‎t/admin/standalone.spec.ts‎

Lines changed: 118 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ import axios from "axios";
1919
import YAML from "yaml";
2020

2121
const ENDPOINT = "/apisix/admin/configs";
22+
const clientConfig = {
23+
baseURL: "http://localhost:1984",
24+
headers: {
25+
"X-API-KEY": "edd1c9f034335f136f87ad84b625c8f1",
26+
},
27+
};
2228
const config1 = {
2329
routes: [
2430
{
@@ -65,12 +71,60 @@ const routeWithModifiedIndex = {
6571
},
6672
],
6773
};
68-
const clientConfig = {
69-
baseURL: "http://localhost:1984",
70-
headers: {
71-
"X-API-KEY": "edd1c9f034335f136f87ad84b625c8f1",
72-
},
73-
};
74+
const routeWithKeyAuth = {
75+
routes: [
76+
{
77+
id: "r1",
78+
uri: "/r1",
79+
upstream: {
80+
nodes: { "127.0.0.1:1980": 1 },
81+
type: "roundrobin",
82+
},
83+
plugins: {
84+
"proxy-rewrite": { uri: "/hello" },
85+
"key-auth": {},
86+
},
87+
},
88+
]
89+
}
90+
const consumerWithModifiedIndex = {
91+
routes: routeWithKeyAuth.routes,
92+
consumers: [
93+
{
94+
modifiedIndex: 10,
95+
username: "jack",
96+
plugins: {
97+
"key-auth": {
98+
key: "jack-key",
99+
}
100+
},
101+
},
102+
],
103+
}
104+
const credential1 = {
105+
routes: routeWithKeyAuth.routes,
106+
consumers: [
107+
{
108+
"username": "john"
109+
},
110+
{
111+
"id": "john/credentials/a",
112+
"plugins": {
113+
"key-auth": {
114+
"key": "auth-a"
115+
}
116+
}
117+
},
118+
{
119+
"id": "john/credentials/b",
120+
"plugins": {
121+
"key-auth": {
122+
"key": "auth-b"
123+
}
124+
}
125+
}
126+
]
127+
}
74128

75129
describe("Admin - Standalone", () => {
76130
const client = axios.create(clientConfig);
@@ -192,14 +246,15 @@ describe("Admin - Standalone", () => {
192246
it('check route "r2"', () =>
193247
expect(client.get("/r2")).rejects.toThrow(
194248
"Request failed with status code 404"
195-
));
249+
));
196250

197251
it("only set routes_conf_version", async () => {
198252
const resp = await client.put(
199253
ENDPOINT,
200254
YAML.stringify({ routes_conf_version: 15 }),
201-
{headers: {"Content-Type": "application/yaml"},
202-
});
255+
{
256+
headers: { "Content-Type": "application/yaml" },
257+
});
203258
expect(resp.status).toEqual(202);
204259

205260
const resp_1 = await client.get(ENDPOINT);
@@ -213,8 +268,9 @@ describe("Admin - Standalone", () => {
213268
const resp2 = await client.put(
214269
ENDPOINT,
215270
YAML.stringify({ routes_conf_version: 17 }),
216-
{headers: {"Content-Type": "application/yaml"},
217-
});
271+
{
272+
headers: { "Content-Type": "application/yaml" },
273+
});
218274
expect(resp2.status).toEqual(202);
219275

220276
const resp2_1 = await client.get(ENDPOINT);
@@ -269,6 +325,48 @@ describe("Admin - Standalone", () => {
269325
const resp3_2 = await client.get("/r2");
270326
expect(resp3_2.status).toEqual(200);
271327
});
328+
329+
it("apply consumer with modifiedIndex", async () => {
330+
const resp = await client.put(ENDPOINT, consumerWithModifiedIndex);
331+
expect(resp.status).toEqual(202);
332+
333+
const resp_1 = await client.get("/r1", { headers: { "apikey": "invalid-key" } }).catch((err) => err.response);
334+
expect(resp_1.status).toEqual(401);
335+
const resp_2 = await client.get("/r1", { headers: { "apikey": "jack-key" } });
336+
expect(resp_2.status).toEqual(200);
337+
338+
const updatedConsumer = structuredClone(consumerWithModifiedIndex);
339+
340+
// update key of key-auth plugin, but modifiedIndex is not changed
341+
updatedConsumer.consumers[0].plugins["key-auth"] = { "key": "jack-key-updated" };
342+
const resp2 = await client.put(ENDPOINT, updatedConsumer);
343+
expect(resp2.status).toEqual(202);
344+
345+
const resp2_1 = await client.get("/r1", { headers: { "apikey": "jack-key-updated" } }).catch((err) => err.response);
346+
expect(resp2_1.status).toEqual(401);
347+
const resp2_2 = await client.get("/r1", { headers: { "apikey": "jack-key" } });
348+
expect(resp2_2.status).toEqual(200);
349+
350+
// update key of key-auth plugin, and modifiedIndex is changed
351+
updatedConsumer.consumers[0].modifiedIndex++;
352+
const resp3 = await client.put(ENDPOINT, updatedConsumer);
353+
const resp3_1 = await client.get("/r1", { headers: { "apikey": "jack-key-updated" } });
354+
expect(resp3_1.status).toEqual(200);
355+
const resp3_2 = await client.get("/r1", { headers: { "apikey": "jack-key" } }).catch((err) => err.response);
356+
expect(resp3_2.status).toEqual(401);
357+
});
358+
359+
it("apply consumer with credentials", async () => {
360+
const resp = await client.put(ENDPOINT, credential1);
361+
expect(resp.status).toEqual(202);
362+
363+
const resp_1 = await client.get("/r1", { headers: { "apikey": "auth-a" } });
364+
expect(resp_1.status).toEqual(200);
365+
const resp_2 = await client.get("/r1", { headers: { "apikey": "auth-b" } });
366+
expect(resp_2.status).toEqual(200);
367+
const resp_3 = await client.get("/r1", { headers: { "apikey": "invalid-key" } }).catch((err) => err.response);
368+
expect(resp_3.status).toEqual(401);
369+
});
272370
});
273371

274372
describe("Exceptions", () => {
@@ -279,18 +377,19 @@ describe("Admin - Standalone", () => {
279377

280378
it("update config (lower conf_version)", async () => {
281379
const resp = await clientException.put(
380+
ENDPOINT,
381+
{ routes_conf_version: 100 },
382+
{ headers: { "Content-Type": "application/yaml" } }
383+
);
384+
const resp2 = await clientException.put(
282385
ENDPOINT,
283386
YAML.stringify(invalidConfVersionConfig1),
284-
{
285-
headers: {
286-
"Content-Type": "application/yaml",
287-
},
288-
}
387+
{ headers: { "Content-Type": "application/yaml" } }
289388
);
290-
expect(resp.status).toEqual(400);
291-
expect(resp.data).toEqual({
389+
expect(resp2.status).toEqual(400);
390+
expect(resp2.data).toEqual({
292391
error_msg:
293-
"routes_conf_version must be greater than or equal to (20)",
392+
"routes_conf_version must be greater than or equal to (100)",
294393
});
295394
});
296395

‎t/cli/test_status_api.sh‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ docker compose -f ./t/cli/docker-compose-etcd-cluster.yaml up -d
4949

5050
make run
5151

52+
sleep 0.5
53+
5254
curl -s -o /dev/null -w "%{http_code}" http://127.0.0.1:7085/status | grep 200 \
5355
|| (echo "failed: status api didn't return 200"; exit 1)
5456

0 commit comments

Comments
 (0)