Kong / kong

🦍 The Cloud-Native API Gateway and AI Gateway.
https://konghq.com/install/#kong-community
Apache License 2.0
39.25k stars 4.82k forks source link

[response-ratelimiting] Missing upstream usage headers in Kong 3.8 #13682

Open t-yuki opened 1 month ago

t-yuki commented 1 month ago

Is there an existing issue for this?

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

  1. In a docker environment,
  2. When I config a service with response-ratelimiting plugin configued with the below for an echo-api upstream server such as https://postman-echo.com/get,
  3. Request the service.
  4. Response doen't contain X-RateLimit-Remaining-Videos header.
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.

ADD-SP commented 1 month ago

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

t-yuki commented 1 month ago

@ADD-SP thanks. I've opened PR https://github.com/Kong/kong/pull/13696