kong icon indicating copy to clipboard operation
kong copied to clipboard

feat(core): support inbound and outbound Via header

Open outsinre opened this issue 1 year ago • 4 comments

Summary

According to RFC7230 and RFC9110 gateway MUST append protocol, version and host (or pseudonym) information when forwarding HTTP messages from clients to upstreams. Currently, Kong just ignores this part.

And optionally append the information when forwarding messages in the reverse direction, depending on the headers configuration. Currently, Kong blindly overrides the response Via header.

This PR will append inbound info like HTTP/1.1 kong/3.8.0, and outbound info like 1.1 kong/3.8.0 (scheme is optional).

This is scheduled for 3.8.0.

Checklist

  • [x] The Pull Request has tests
  • [x] A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • [ ] There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

Fix #FTI-5807

outsinre avatar Mar 14 '24 13:03 outsinre

Will add tests to spec/02-integration/05-proxy/03-upstream_headers_spec.lua

outsinre avatar Mar 14 '24 13:03 outsinre

Latest RFC: https://datatracker.ietf.org/doc/html/rfc9110#name-via

bungle avatar Mar 14 '24 22:03 bungle

This is for 3.8.0. Please do not merge it until 3.7.0 GA.

outsinre avatar Apr 03 '24 03:04 outsinre

This is for 3.8.0 as Paypal (customer candidate) may have further requirements.

outsinre avatar Apr 07 '24 03:04 outsinre

Just did a rebase and waiting for CI.

outsinre avatar Jul 03 '24 03:07 outsinre

A couple of comments:

  1. I would love to see tests added where we have 2.0 as protocol version in incoming (http2/grpc) and 2.0 on upstream when doing grpc_pass (not sure about 1.0, should we test it too). (a real integration test)
  2. this has long been how Kong does it, so it has kinda become a "feature", thus fixing it according to RFC may cause breakage in customers systems, e.g. those that expect these headers to only ever have single value. so just as a not, not sure if that is enough to post-pone this to 4.0 (perhaps product can answer).
  3. additional breakage is proxy_set_header Via $upstream_via;, so any plugin that sets Via currently does not work anymore as they will get overridden (again I am not sure if that is a bad thing, just pointing out)

bungle avatar Jul 04 '24 08:07 bungle

@outsinre also things like these: https://github.com/Kong/kong/blob/master/kong/plugins/aws-lambda/handler.lua#L240-L242

bungle avatar Jul 04 '24 09:07 bungle

Successfully created cherry-pick PR for master:

  • https://github.com/kong/kong-ee/pull/9716

team-gateway-bot avatar Jul 15 '24 06:07 team-gateway-bot