change: global rules should not be executed on the internal api#3396
change: global rules should not be executed on the internal api#3396spacewander merged 6 commits intoapache:masterfrom
Conversation
| end | ||
| end | ||
|
|
||
| if router.api.has_route_not_under_apisix() or |
There was a problem hiding this comment.
Maybe we can add a configuration in the config.yaml for it?
There was a problem hiding this comment.
@spacewander
I thought about it again. There shouldn't have a switch here. If skip the if branch and continue execution, a 404 will occur because the route does not exist.
And I think if need to configure the plugin for the internal api, it is more appropriate to configure a route and then configure the plugin instead of using the global rule.
What do you think ? Looking forward to your opinion. Thanks.
There was a problem hiding this comment.
Better to provide a switch to execute the global rule before matching internal API or don't execute the global rule. It is not skipped the internal API matched.
And I think if need to configure the plugin for the internal api, it is more appropriate to configure a route and then configure the plugin instead of using the global rule.
We can't configure a plugin for internal API, this is the real problem.
There was a problem hiding this comment.
@spacewander Thanks for for the explanation. I know that.., emm, will update later.
spacewander
changed the title
|
We need a test case for it. |
we have test case for it, : ) |
| end | ||
| end | ||
|
|
||
| if router.api.has_route_not_under_apisix() or |
apisix/init.lua
Outdated
| if router.api.has_route_not_under_apisix() or | ||
| core.string.has_prefix(uri, "/apisix/") | ||
| then | ||
| matched_internal_api = router.api.match(api_ctx) |
There was a problem hiding this comment.
The internal API will call exit inside so there is not change to execute the global rule.
Maybe we can refactor and exact the global rule code, then run it before
Line 124 in c895cdf
| role: viewer | ||
|
|
||
| delete_uri_tail_slash: false # delete the '/' at the end of the URI | ||
| global_rule_skip_internal_api: true # does not run global rule in internal apis |
There was a problem hiding this comment.
Can we add test when global_rule_skip_internal_api is false?
There was a problem hiding this comment.
We need to add test when global_rule_skip_internal_api is false.
There was a problem hiding this comment.
We need to add test when global_rule_skip_internal_api is false.
sure, test failed now, submit it after solving it.
There was a problem hiding this comment.
@spacewander teat case added. please help review when you have time, thanks.
4 participants
What this PR does / why we need it:
global rules should not be executed on the internal api, currently it will run
accessandrewriteof plugins in global rule, so we need to change it.Pre-submission checklist: