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

tests(*): remove resty luasocket #12668

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

windmgc
Copy link
Member

@windmgc windmgc commented Feb 29, 2024

Summary

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]

@bungle
Copy link
Member

bungle commented Feb 29, 2024

@windmgc, I think only EE patches the running environment in init phase with resty.luasocket.http thus you see it failing here. Though I am not sure what installs the library in CE CI. It is a bit unexpected that it works here. Perhaps it is a dependency of a dependency, and that's how we get it. If that is true, we should rather be explicit about it. Probably from here: https://github.com/Kong/lua-resty-aws/blob/main/lua-resty-aws-dev-1.rockspec.template#L28 (and similar libs).

@windmgc
Copy link
Member Author

windmgc commented Feb 29, 2024

a dependency of a dependency

I believe it is the truth. So I'm checking whether we can remove all of these(including the globalpatch in EE), since we already solved the CLI trusted certificate issue by injecting configs for resty CLI in 3.5

@windmgc
Copy link
Member Author

windmgc commented Feb 29, 2024

Oh, I forget that cosocket is also not available in init_phase, so resty.http cannot be directly used

2024/02/29 21:22:56 [error] 26418#0: init_by_lua error: ./kong/globalpatches.lua:574: no request found
stack traceback:
        [C]: in function 'old_tcp'
        ./kong/globalpatches.lua:574: in function 'ngx_socket_tcp'
        ...ng/bazel-bin/build/kong-dev/share/lua/5.1/resty/http.lua:133: in function 'new'
        ...c/fixtures/custom_vaults/kong/vaults/mocksocket/init.lua:17: in function 'get'
        ./kong/pdk/vault.lua:721: in function 'invoke_strategy'
        ./kong/pdk/vault.lua:805: in function 'get'
        ./kong/pdk/vault.lua:1677: in function 'warmup'
        ./kong/pdk/vault.lua:1675: in function 'warmup'
        ./kong/pdk/vault.lua:1675: in function 'warmup'
        ./kong/pdk/vault.lua:1675: in function 'warmup'
        ./kong/init.lua:721: in function 'init'
        init_by_lua(nginx-kong.conf:41):3: in main chunk

So it is still useful and cannot be removed...

@bungle
Copy link
Member

bungle commented Mar 5, 2024

So it is still useful and cannot be removed...

Allowing co-sockets in init AND init_worker would be great. Though init_worker doesn't even support couroutine.* (which init does). But getting cosockets in init would be a great start.

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.

2 participants