[response-ratelimiting] Missing upstream usage headers in Kong 3.8
Is there an existing issue for this?
- [X] I have searched the existing issues
Kong version ($ kong version)
Kong 3.8.0.0
Current Behavior
No usage headers such as X-RateLimit-Remaining-Videos: 10 for upstream requests.
Expected Behavior
When I do request the api, X-RateLimit-Remaining-Videos: 10 header should be present in upstream request.
Steps To Reproduce
- In a docker environment,
- When I config a service with
response-ratelimitingplugin configued with the below for an echo-api upstream server such ashttps://postman-echo.com/get, - Request the service.
- Response doen't contain
X-RateLimit-Remaining-Videosheader.
plugins:
- name: response-ratelimiting
config:
limits:
videos:
minute: 10
policy: local
Anything else?
According to the plugin document page https://docs.konghq.com/hub/kong-inc/response-ratelimiting/#upstream-headers , the plugin should append the usage headers for each limit before proxying it to the upstream service, but it is missing on Kong 3.8
I guess the change in https://github.com/Kong/kong/commit/3c0aa6097f1421e2d0bfd4bc7c8be8fa54a5b881#diff-e799a72960fa7f956455fff4cca7947749b5c6ecd7cb1e18f220931911eb0bcfR106 caused this behaivior.
Before Kong 3.8, it was set by kong.service.request.set_header function so upstream requests contain remaining usage headers.
But in this change, it was replaced by pdk_rl_store_response_header and pdk_rl_apply_response_headers functions those finally manipulates ngx.header https://github.com/Kong/kong/blob/3.8.0/kong/pdk/private/rate_limiting.lua#L7 , that is client response headers, not request headers for upstream.
Thanks, @t-yuki, I agree with your analysis, and we welcome this fix from the community. Are you willing to open a pull request to fix it?
Internal ticket: KAG-5447
@ADD-SP thanks. I've opened PR https://github.com/Kong/kong/pull/13696
@ADD-SP thanks. I've opened PR #13696
Hi @t-yuki , I have some comments on https://github.com/Kong/kong/pull/13696, please review it in your convenience, that would help us merge the PR faster.
@ProBrian Excuse me, I'm unable to find your comments in #13696 .
@ProBrian Excuse me, I'm unable to find your comments in #13696 .
Sorry I forgot to submit the review, please check again, thanks. @t-yuki