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

separate plugin server runtime data from kong configuration #14111

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ProBrian
Copy link
Contributor

@ProBrian ProBrian commented Jan 8, 2025

Summary

Problem: There are some runtime data like proc stored in kong.configuration.pluginservers, which is cdata type and could not be encoded by cjson. Thus when fetching the admin api "/", the response body encoding of configuration will fail and throws 500 error.
Solution: Separate runtime data from configuration, store runtime info like proc inside the plugin servers module, not global configuration.

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 https://konghq.atlassian.net/browse/KAG-6088

@ProBrian ProBrian marked this pull request as draft January 8, 2025 08:52
@ProBrian ProBrian requested a review from gszr January 9, 2025 01:06
@ProBrian ProBrian marked this pull request as ready for review January 9, 2025 01:16
@@ -150,16 +151,19 @@ local function pluginserver_timer(premature, server_def)
end

kong.log.notice("[pluginserver] starting pluginserver process for ", server_def.name or "")
server_def.proc = assert(ngx_pipe.spawn("exec " .. server_def.start_command, {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the server_ref passed in here is the reference of kong.configuration.pluginservers, so the proc(which is just a runtime sub process handle) here will be included in configuration, which is not reasonable, and also causes encoding of configuration failed on admin API GET method.

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

Successfully merging this pull request may close these issues.

1 participant