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(plugins): load wasm filter plugins before external plugins #13931

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

flrgh
Copy link
Contributor

@flrgh flrgh commented Nov 26, 2024

Summary

In 4059a31 (#13843) we added a plugin-like interface for Wasm filters.

We now have 3 sources for plugins: Lua, External, and Wasm Filters. When a plugin is enabled or configured, the plugin system follows a resolution order for looking up the plugin handler and schema:

  1. Lua => require kong.plugins.<name>.{handler,schema}
  2. External => kong.runloop.plugin_servers.load_{handler,schema}(<name>)
  3. Wasm Filters => kong.runloop.wasm.plugins.load_{handler,schema}(<name>)

When a user configures Kong with a "bad" entry in pluginserver_names (read: a plugin server that is not actually installed), step #2 of the plugin resolution process throws an exception, because the external plugin subsystem attempts to query a plugin server that does not exist. Importantly, this exception is not encountered when the user has only configured/enabled Lua plugins, because we never reach beyond step #1 of the plugin resolution process.

A side effect of adding the Wasm filter plugin interface is that discovered Wasm filters are added to the global plugins table (kong.configuration.loaded_plugins) when Wasm is enabled. This means that, if Wasm is enabled, and any Wasm filters are installed, we always step through step #2 of the plugin resolution process, triggering an exception if the user has any badly-configured plugin server.

A future change will likely render this scenario unreachable by performing deeper validation of pluginserver_names at startup. For now, a simple fix is just to change the resolution order such that Wasm filters are loaded before we query the external plugin subsystem:

  1. Lua
  2. Wasm Filters
  3. External

Checklist

  • The Pull Request has tests
  • A changelog file has been created (this fixes an unreleased bug)
  • There is a user-facing docs PR

Issue reference

KAG-5911

@flrgh flrgh added this to the 3.9.0 milestone Nov 26, 2024
@flrgh flrgh requested review from gszr and locao November 26, 2024 19:54
@github-actions github-actions bot added core/db schema-change-noteworthy cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Nov 26, 2024
@flrgh flrgh added skip-changelog and removed cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Nov 26, 2024
@flrgh
Copy link
Contributor Author

flrgh commented Nov 26, 2024

note: I removed the cherry-pick label in favor of manually cherry-picking this into the in-progress branch in EE land:

https://github.com/Kong/kong-ee/pull/10742

In 4059a31 (#13843) we added a
plugin-like interface for Wasm filters.

We now have 3 sources for plugins: Lua, External, and Wasm Filters. When a plugin is enabled or configured, the plugin system follows a
resolution order for looking up the plugin handler and schema:

1. Lua => `require kong.plugins.<name>.{handler,schema}`
2. External => `kong.runloop.plugin_servers.load_{handler,schema}(<name>)`
3. Wasm Filters => `kong.runloop.wasm.plugins.load_{handler,schema}(<name>)`

When a user configures Kong with a "bad" entry in `pluginserver_names`
(read: a plugin server that is not actually installed), step #2 of the
plugin resolution process throws an exception, because the external
plugin subsystem attempts to query a plugin server that does not exist.
Importantly, *this exception is not encountered when the user has only
configured/enabled Lua plugins,* because we never reach beyond step #1
of the plugin resolution process.

A side effect of adding the Wasm filter plugin interface is that
discovered Wasm filters are added to the global plugins table
(`kong.configuration.loaded_plugins`) when Wasm is enabled. This means
that, if Wasm is enabled, and any Wasm filters are installed, we
_always_ step through step #2 of the plugin resolution process,
triggering an exception if the user has any badly-configured plugin
server.

A future change will likely render this scenario unreachable by
performing deeper validation of `pluginserver_names` at startup. For
now, a simple fix is just to change the resolution order such that Wasm
filters are loaded _before_ we query the external plugin subsystem:

1. Lua
2. Wasm Filters
3. External
@github-actions github-actions bot added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Nov 26, 2024
@locao locao removed the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Nov 26, 2024
@flrgh
Copy link
Contributor Author

flrgh commented Nov 26, 2024

I'm going to wait to merge this until after the EE PR is green just to ensure there are no other fixes needed here.

@gszr gszr merged commit b1cf758 into master Nov 27, 2024
31 checks passed
@gszr gszr deleted the fix/wasm-and-pluginserver branch November 27, 2024 00:15
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