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

[backport -> release/3.6.x] fix(wasm): do not call attach() on re-entrancy #12452

Merged
merged 1 commit into from Jan 29, 2024

Conversation

team-gateway-bot
Copy link
Collaborator

Automated backport to release/3.6.x, triggered by a label in #12402.

Original description

Summary

kong.runloop.wasm.attach() may be called more than once when filters utilize the dispatch_http_call proxy-wasm SDK function. Previously this was assumed to be false, which can yield errors like this one:

2024-01-22 16:39:16 2024/01/22 22:39:16 [crit] 2464#0: *340 [lua] wasm.lua:790: attach(): failed attaching service(6bc44ccb-30c9-5f3d-acfa-017fa1b2ef82) filter chain to request: previous plan already attached, client: 172.17.0.1, server: kong, request: "GET /anything HTTP/1.1", host: "localhost:8000", request_id: "112ec236dcf487391a579a6f6f63fc0b"

This adds a ngx.ctx check and avoids calling proxy_wasm.attach() in this case.

Unfortunately it is incredibly non-trivial to add a test case for this at the moment, but the behavior change/correctness has been validated by hand.

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

Issue reference

KAG-3603

@team-gateway-bot team-gateway-bot added this to the 3.6.0 milestone Jan 29, 2024
@team-gateway-bot team-gateway-bot added cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/proxy core/wasm Everything relevant to [proxy-]wasm size/M labels Jan 29, 2024
@flrgh flrgh requested review from flrgh and locao January 29, 2024 22:01
@locao
Copy link
Contributor

locao commented Jan 29, 2024

Waiting for a green run before merging.

@flrgh flrgh merged commit f85129c into release/3.6.x Jan 29, 2024
44 checks passed
@flrgh flrgh deleted the backport-12402-to-release/3.6.x branch January 29, 2024 22:23
@team-gateway-bot
Copy link
Collaborator Author

Cherry-pick failed for release/3.6.x: couldn't find remote ref release/3.6.x.
Please ensure that this Github repo has a branch named release/3.6.x.

@team-gateway-bot
Copy link
Collaborator Author

team-gateway-bot commented Jan 29, 2024

Cherry-pick failed for release/3.6.x, because it was unable to create a new branch.

Please cherry-pick the changes locally.

git remote add upstream https://github.com/kong/kong-ee
git fetch upstream release/3.6.x
git worktree add -d .worktree/cherry-pick-12452-to-release/3.6.x-to-upstream upstream/release/3.6.x
cd .worktree/cherry-pick-12452-to-release/3.6.x-to-upstream
git checkout -b cherry-pick-12452-to-release/3.6.x-to-upstream
ancref=$(git merge-base 6cf74cdabf97e672569f3be9b3ec502be6b56219 1edb16b1349c796bfe55ae0553a4712d4a703305)
git cherry-pick -x $ancref..1edb16b1349c796bfe55ae0553a4712d4a703305

edit (@flrgh): backporting from this PR instead

This was referenced Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/proxy core/wasm Everything relevant to [proxy-]wasm size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants