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(wasm/config): correctly map inherited directives #11251

Merged
merged 2 commits into from
Jul 19, 2023

Conversation

gszr
Copy link
Member

@gszr gszr commented Jul 19, 2023

Summary

This addresses some of the outstanding feedback from @thibaultcha on the Wasm integration PR.

Copy link
Member

@thibaultcha thibaultcha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for promptly addressing those!

kong/conf_loader/init.lua Show resolved Hide resolved
@gszr gszr force-pushed the fix/wasm-inherited-directives branch from 63def32 to 164b049 Compare July 19, 2023 15:37
@gszr gszr force-pushed the fix/wasm-inherited-directives branch from 164b049 to 4d82a90 Compare July 19, 2023 19:48
@gszr gszr requested a review from flrgh July 19, 2023 19:48
@RobSerafini RobSerafini added this to the 3.4.0 milestone Jul 19, 2023
kong/conf_loader/init.lua Outdated Show resolved Hide resolved
@gszr gszr force-pushed the fix/wasm-inherited-directives branch from 4d82a90 to 2100a00 Compare July 19, 2023 20:22
@gszr gszr requested a review from flrgh July 19, 2023 20:23
Copy link
Contributor

@flrgh flrgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a test lint thing to fix, but otherwise this looks good to me! 👍

gszr added 2 commits July 19, 2023 19:18
The Kong Wasm integration inherits certain configuration properties
defined for Kong; among others, it inherits injected Nginx directives.
As implemented in #11218, the
integration inherited certain directives from the `ngx_http_proxy_module`,
while  the desired behavior is to inherit from the `ngx_lua_module`, so
that Lua and Wasm extensions operate under the same settings. This commit
implements that change.
Remove warning when `wasm=off` and Wasm directives are set. The team
agrees such a warning does not provide much value as the user may toggle
Wasm on and off temporarily for testing.
@gszr gszr force-pushed the fix/wasm-inherited-directives branch from eb33df4 to d2f6969 Compare July 19, 2023 22:18
@gszr gszr merged commit 9942c71 into master Jul 19, 2023
20 checks passed
@gszr gszr deleted the fix/wasm-inherited-directives branch July 19, 2023 23:10
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.

4 participants