hanzhenfang commented on issue #13357:
URL: https://github.com/apache/apisix/issues/13357#issuecomment-4645747083
Hi @Baoyuantop , I was able to reproduce this locally as well. Sharing the
minimal setup and a bit of context in case it helps with reviewing or
validating the fix.
The repro intentionally exercises the defensive branch in
`limit-count/init.lua` by calling `limit_count.rate_limit()` with a manually
constructed config whose `_meta.parent` is missing. This is not meant to show
that a normal Admin API / route configuration naturally loses `_meta.parent`;
it is just a minimal way to trigger the exact `error()` call reported in this
issue.
Minimal standalone repro:
`docker-compose.yaml`
```yaml
services:
apisix:
image: apache/apisix:3.16.0-debian
container_name: apisix-issue-13357
depends_on:
- upstream
volumes:
- ./config.yaml:/usr/local/apisix/conf/config.yaml:ro
- ./apisix.yaml:/usr/local/apisix/conf/apisix.yaml:ro
ports:
- target: 9080
host_ip: 127.0.0.1
protocol: tcp
upstream:
image: nginx:alpine
container_name: apisix-issue-13357-upstream
```
`config.yaml`
```yaml
deployment:
role: data_plane
role_data_plane:
config_provider: yaml
apisix:
node_listen: 9080
plugins:
- serverless-pre-function
- limit-count
```
`apisix.yaml`
```yaml
routes:
- id: issue-13357
uri: /issue-13357
plugins:
serverless-pre-function:
phase: access
functions:
- |
return function(conf, ctx)
local limit_count =
require("apisix.plugins.limit-count.init")
local broken_limit_count_conf = {
policy = "local",
count = 5,
time_window = 60,
key = "remote_addr",
key_type = "var",
rejected_code = 503,
allow_degradation = false,
show_limit_quota_header = true,
_meta = {}
}
return limit_count.rate_limit(broken_limit_count_conf, ctx,
"limit-count", 1)
end
upstream:
type: roundrobin
nodes:
"upstream:80": 1
#END
```
Then run:
```bash
cd issue-13357-limit-count-error-repro
docker compose up -d
APISIX_URL="http://$(docker compose port apisix 9080)"
curl -i "$APISIX_URL/issue-13357"
docker compose logs --tail=120 apisix
```
On the current code, the request returns `500`, and the APISIX log shows:
```text
/usr/local/apisix/apisix/plugins/limit-count/init.lua:262: bad argument #2
to 'error' (number expected, got string)
stack traceback:
[C]: in function 'error'
/usr/local/apisix/apisix/plugins/limit-count/init.lua:262: in function
'gen_limit_key'
/usr/local/apisix/apisix/plugins/limit-count/init.lua:398: in function
'run_rate_limit'
```
With the proposed change, I would expect the request to still fail, but with
the intended message instead:
```text
failed to generate key invalid parent: null
```
One extra context point: in the normal route/service/plugin_config loading
path, APISIX should inject `conf._meta.parent.resource_key` via
`set_plugins_meta_parent()`. So I think this issue is specifically about making
the defensive error path report the intended message, while any real
execution path that loses `_meta.parent.resource_key` would probably be a
separate issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
