Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(pdk): get the first http header value after bumping to OpenResty 1.25.3.1 #12370

Merged
merged 11 commits into from
Jan 24, 2024

Conversation

chronolaw
Copy link
Contributor

@chronolaw chronolaw commented Jan 18, 2024

Summary

It is a fixing PR for #12327.
KAG-3570

Checklist

  • The Pull Request has tests
  • 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 #[issue number]

@chronolaw chronolaw changed the title fix(pdk): get the first http header value in OpenResty 1.25.3.1 fix(pdk): get the first http header value after bumping to OpenResty 1.25.3.1 Jan 18, 2024
@chronolaw chronolaw added this to the 3.6.0 milestone Jan 18, 2024
@chronolaw chronolaw marked this pull request as ready for review January 19, 2024 01:28
@chronolaw chronolaw requested review from bungle and dndx January 19, 2024 01:28
kong/tools/http.lua Outdated Show resolved Hide resolved
@bungle
Copy link
Member

bungle commented Jan 19, 2024

@chronolaw we need to somehow also handle the header modification case too?

This is how our current code works I think:

kong.service.request.set_header("a", "b")
local a = kong.request.get_header("a")
kong.service.request.set_header("a", "c")
local a2 = kong.request.get_header("a")
assert(a ~= a2) 

It would though be possible with this new ctx thing to have client request headers separated from upstream request headers, but that is not what we currently do, but something to think about perhaps, we already have:

kong.service.response.get_header()
kong.response.get_header()

That do separate service response headers from kong response headers (kong response headers sure contain the upstream response headers too, but they may contain modifications/additions too).

Perhaps the cache thing needs to be in those global patches.

@chronolaw
Copy link
Contributor Author

If we have an assumption that multiple header value is not a frequent case when accessing by ngx.var,
could we not cache the result of get_headers() in ctx table?

@chronolaw chronolaw force-pushed the fix/get_one_http_header_value branch from 926b4f6 to 0d58604 Compare January 20, 2024 00:38
@chronolaw chronolaw force-pushed the fix/get_one_http_header_value branch from a539a7e to b5f9bca Compare January 20, 2024 12:02
dndx
dndx previously requested changes Jan 22, 2024
kong/tools/http.lua Outdated Show resolved Hide resolved
@dndx dndx requested a review from bungle January 22, 2024 07:23
kong/pdk/request.lua Outdated Show resolved Hide resolved
@bungle bungle dismissed dndx’s stale review January 24, 2024 10:16

changed accordingly.

@bungle bungle added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Jan 24, 2024
@bungle bungle merged commit 14cc90f into master Jan 24, 2024
28 checks passed
@bungle bungle deleted the fix/get_one_http_header_value branch January 24, 2024 10:18
@team-gateway-bot
Copy link
Collaborator

Cherry-pick failed for master, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git remote add upstream https://github.com/kong/kong-ee
git fetch upstream master
git worktree add -d .worktree/cherry-pick-12370-to-master-to-upstream upstream/master
cd .worktree/cherry-pick-12370-to-master-to-upstream
git checkout -b cherry-pick-12370-to-master-to-upstream
ancref=$(git merge-base 58fe2dd31c06a1f2910c2b69deb678f39c573177 1198c8132c4009849eeaec9bdbf134f13eab767b)
git cherry-pick -x $ancref..1198c8132c4009849eeaec9bdbf134f13eab767b

@chronolaw chronolaw removed the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Jan 24, 2024
@chronolaw
Copy link
Contributor Author

Only when the cherry-pick of #12327 is merged we will cherry pick this one.

@chronolaw chronolaw added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Jan 24, 2024
@team-gateway-bot
Copy link
Collaborator

Cherry-pick failed for master, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git remote add upstream https://github.com/kong/kong-ee
git fetch upstream master
git worktree add -d .worktree/cherry-pick-12370-to-master-to-upstream upstream/master
cd .worktree/cherry-pick-12370-to-master-to-upstream
git checkout -b cherry-pick-12370-to-master-to-upstream
ancref=$(git merge-base 58fe2dd31c06a1f2910c2b69deb678f39c573177 1198c8132c4009849eeaec9bdbf134f13eab767b)
git cherry-pick -x $ancref..1198c8132c4009849eeaec9bdbf134f13eab767b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants