fix(cmd): lack of necessary nginx directives in kong cli nginx.conf #11127

catbro666 · 2023-06-27T08:24:28Z

Summary

This is an alternative of #10675. The primary logic keeps the same, but the inject logic is further moved forward from kong/cmd/init.lua to bin/kong so that the execution flow won't enter kong/cmd/init.lua twice.

We still keep the bin/kong a resty script because many files such as kong.conf_loader, kong.cmd.utils.process_secrets rely on ngx. If we change bin/kong into a pure lua or other language script, we need to rewrite the conf_loader and compile part logic.

FTI-4937

Checklist

The Pull Request has tests
There's an entry in the CHANGELOG

spec/01-unit/03-conf_loader_spec.lua

catbro666 · 2023-07-05T09:24:44Z

detach the "drop luasocket in cli" part to a separate PR so that this PR is easier to review. Also because these two PRs are relatively logically independent.

bin/kong

kikito · 2023-07-11T17:10:54Z

Is this needed for 3.4 (it's in the milestone)?

catbro666 · 2023-07-12T02:47:51Z

Is this needed for 3.4 (it's in the milestone)?

@kikito Yes, I think so. it was moved from 3.3 to 3.4 because of pursuing a more generic approach.

We, @kikito, @jschmid1, and @bungle, decided to move this to 3.4. This is a rather generic problem, and we would like to pursue different approaches before committing to this. Approaches that might work in more generic fashion are:

Co-authored-by: Qirui(Keery) Nie <[email protected]>

the lua_ssl_trusted_certificate config may be updated

Already addressed

* fix(cmd): `kong vault get` doesn't work in dbless mode The cli `kong vault get <reference>` doesn't work in DBless mode if <reference> uses vaults entity. It doesn't affect the normal use of vault in kong instance though. The reason is in DBless mode the vaults entity is stored in LMDB which is implemented by a Nginx C module. However Everytime `resty` cli (which is relied on by `kong` cli) runs it creates a temporary `nginx.conf` which doesn't contain the lmdb-related directives. This PR is fixing this by starting another `resty` call with lmdb-related directives inserted via the `--main-conf` option. Note we only try this after detecting the `no LMDB environment defined` error in order to avoid infinite loop. And because `resty` will create a temmporary nginx instance so we need to convert the relative paths in the nginx.conf to the absolute path under kong instance prefix. [FTI-4937](https://konghq.atlassian.net/browse/FTI-4937) * add CHANGELOG * make it more robust * update comment * update comment * test the existence of LMDB rather than Kong instance * fixup * make the fix more generic * fix and add tests in 04-prefix_handler_spec * add lua_ssl_protocols and fix tests * rename the new configuration files to avoid conflict with the prefix of injected directives * add and fix tests of 14-vault_spec * fix test * rename template files to consistent with configuration file names * add unit tests for inject_directives.lua * change to absolute path * fixup * fix path * Update CHANGELOG.md Co-authored-by: Hans Hübner <[email protected]> * use return (...) syntax instead * don't expose the option and use a better name * pass paths instead of patterns and use better names * correctly handle the stdout/stderr/exit code * preserve original cli args for reusing * use env variable to terminate recursion * resty isn't necessarily in the position -1, so add it explicitly * update the lmdb_map_size to 2048m * fix(cmd): lack of necessary nginx directives in kong cli nginx.conf This is an alternative of (#10675)[#10675]. The primary logic keeps the same. The inject logic is further moved forward from `kong/cmd/init.lua` to `bin/kong` so that the execution flow won't enter `kong/cmd/init.lua` twice. We still keep the `bin/kong` a resty script because many files such as `kong.conf_loader`, `kong.cmd.utils.process_secrets` rely on `ngx`. If we change `bin/kong` into a pure lua or other language script, we need to rewrite the conf_loader and compile part logic. [FTI-4937](https://konghq.atlassian.net/browse/FTI-4937) * fix lint * fix test * fix test * use xpcall to catch exceptions and handle error message * add health to skip_inject_cmds * fix tests in 11-config_spec.lua * add hybrid into skip_inject_cmds * fix typo * remove CHANGELOG entry to the right place ("Unreleased") * extend load() to a subset of fields and these fields can't reference vault * add field `database` to CONF_NO_VAULT * fix test * fix test * keep `conf.nginx_http_lua_ssl_protocols` and `conf.nginx_stream_lua_ssl_protocols` so that we don't change the previous behavior * fixup * fix test * fix test * fix test * update CHANGELOG * Update CHANGELOG.md Co-authored-by: Qirui(Keery) Nie <[email protected]> * always call prepare_prefix as the prefix directory may not existed and the lua_ssl_trusted_certificate config may be updated --------- Co-authored-by: Hans Hübner <[email protected]> Co-authored-by: Qirui(Keery) Nie <[email protected]> (cherry picked from commit 8a1ebba)

This is the follow-up PR of #11127 Changing the socket type from luasocket to openresty cosocket causes some test fail weirdly. After investigating, it's mainly because the cosocket support yield and setkeepalive. See the comments in tests. https://konghq.atlassian.net/browse/FTI-4937

* fix(cmd): `kong vault get` doesn't work in dbless mode The cli `kong vault get <reference>` doesn't work in DBless mode if <reference> uses vaults entity. It doesn't affect the normal use of vault in kong instance though. The reason is in DBless mode the vaults entity is stored in LMDB which is implemented by a Nginx C module. However Everytime `resty` cli (which is relied on by `kong` cli) runs it creates a temporary `nginx.conf` which doesn't contain the lmdb-related directives. This PR is fixing this by starting another `resty` call with lmdb-related directives inserted via the `--main-conf` option. Note we only try this after detecting the `no LMDB environment defined` error in order to avoid infinite loop. And because `resty` will create a temmporary nginx instance so we need to convert the relative paths in the nginx.conf to the absolute path under kong instance prefix. [FTI-4937](https://konghq.atlassian.net/browse/FTI-4937) * add CHANGELOG * make it more robust * update comment * update comment * test the existence of LMDB rather than Kong instance * fixup * make the fix more generic * fix and add tests in 04-prefix_handler_spec * add lua_ssl_protocols and fix tests * rename the new configuration files to avoid conflict with the prefix of injected directives * add and fix tests of 14-vault_spec * fix test * rename template files to consistent with configuration file names * add unit tests for inject_directives.lua * change to absolute path * fixup * fix path * Update CHANGELOG.md Co-authored-by: Hans Hübner <[email protected]> * use return (...) syntax instead * don't expose the option and use a better name * pass paths instead of patterns and use better names * correctly handle the stdout/stderr/exit code * preserve original cli args for reusing * use env variable to terminate recursion * resty isn't necessarily in the position -1, so add it explicitly * update the lmdb_map_size to 2048m * fix(cmd): lack of necessary nginx directives in kong cli nginx.conf This is an alternative of (#10675)[#10675]. The primary logic keeps the same. The inject logic is further moved forward from `kong/cmd/init.lua` to `bin/kong` so that the execution flow won't enter `kong/cmd/init.lua` twice. We still keep the `bin/kong` a resty script because many files such as `kong.conf_loader`, `kong.cmd.utils.process_secrets` rely on `ngx`. If we change `bin/kong` into a pure lua or other language script, we need to rewrite the conf_loader and compile part logic. [FTI-4937](https://konghq.atlassian.net/browse/FTI-4937) * fix lint * fix test * fix test * use xpcall to catch exceptions and handle error message * add health to skip_inject_cmds * fix tests in 11-config_spec.lua * add hybrid into skip_inject_cmds * fix typo * remove CHANGELOG entry to the right place ("Unreleased") * extend load() to a subset of fields and these fields can't reference vault * add field `database` to CONF_NO_VAULT * fix test * fix test * keep `conf.nginx_http_lua_ssl_protocols` and `conf.nginx_stream_lua_ssl_protocols` so that we don't change the previous behavior * fixup * fix test * fix test * fix test * update CHANGELOG * Update CHANGELOG.md Co-authored-by: Qirui(Keery) Nie <[email protected]> * always call prepare_prefix as the prefix directory may not existed and the lua_ssl_trusted_certificate config may be updated --------- Co-authored-by: Hans Hübner <[email protected]> Co-authored-by: Qirui(Keery) Nie <[email protected]> (cherry picked from commit 8a1ebba)

allowd to reference vaults introduced by #11127 [FTI-4937](https://konghq.atlassian.net/browse/FTI-4937)

…#11291) This commit allows some configuration fields to be referenced by using vaults. The limitation is introduced by #11127, and this commit removes the limitation to keep the behaviour to be the same as before [FTI-4937](https://konghq.atlassian.net/browse/FTI-4937)

…#11291) This commit allows some configuration fields to be referenced by using vaults. The limitation is introduced by #11127, and this commit removes the limitation to keep the behaviour to be the same as before [FTI-4937](https://konghq.atlassian.net/browse/FTI-4937) (cherry picked from commit 522f554)

This is the follow-up PR of #11127 Changing the socket type from luasocket to openresty cosocket causes some test fail weirdly. After investigating, it's mainly because the cosocket support yield and setkeepalive. See the comments in tests. https://konghq.atlassian.net/browse/FTI-4937

This is the follow-up PR of #11127 Changing the socket type from luasocket to openresty cosocket causes some test fail weirdly. After investigating, it's mainly because the cosocket support yield and setkeepalive. See the comments in tests. FTI-4937

team-gateway-bot · 2023-09-25T13:34:15Z

The backport to release/3.3.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-release/3.3.x release/3.3.x
# Navigate to the new working tree
cd .worktrees/backport-release/3.3.x
# Create a new branch
git switch --create backport-11127-to-release/3.3.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 8a1ebba055d28f940cc19774cf5edf35f71dd149
# Push it to GitHub
git push --set-upstream origin backport-11127-to-release/3.3.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-release/3.3.x

Then, create a pull request where the base branch is release/3.3.x and the compare/head branch is backport-11127-to-release/3.3.x.

kikito · 2023-09-25T13:41:24Z

Backporting this to 3.3 since it seems this is needed in #11210 (failing test complains about missing inject_confs file)

pull-request-size bot added the size/L label Jun 27, 2023

github-actions bot added the core/cli label Jun 27, 2023

catbro666 changed the base branch from FTI-4937-kong-cli-vault-caveats to master June 27, 2023 08:25

pull-request-size bot added size/XL and removed size/L labels Jun 27, 2023

catbro666 force-pushed the bin-kong-to-lua-script branch from f941ddf to 97a7d26 Compare June 27, 2023 08:27

github-actions bot added changelog core/configuration core/templates labels Jun 27, 2023

catbro666 added this to the 3.4.0 milestone Jun 27, 2023

bungle previously requested changes Jun 27, 2023

View reviewed changes

spec/01-unit/03-conf_loader_spec.lua Outdated Show resolved Hide resolved

catbro666 force-pushed the bin-kong-to-lua-script branch 2 times, most recently from 89b530f to 16526fb Compare June 29, 2023 03:35

github-actions bot added the core/db label Jun 29, 2023

catbro666 force-pushed the bin-kong-to-lua-script branch from ecba305 to 03459b1 Compare June 29, 2023 04:31

github-actions bot added core/db and removed core/db labels Jun 29, 2023

catbro666 force-pushed the bin-kong-to-lua-script branch 2 times, most recently from 0f74d8c to 533c329 Compare July 5, 2023 08:22

catbro666 mentioned this pull request Jul 5, 2023

fix(*): drop luasocket in cli #11177

Merged

catbro666 force-pushed the bin-kong-to-lua-script branch from 533c329 to 5effcad Compare July 5, 2023 09:15

github-actions bot removed the core/db label Jul 5, 2023

catbro666 requested a review from bungle July 5, 2023 09:49

catbro666 added the pr/please review label Jul 5, 2023

catbro666 requested review from fffonion and windmgc July 5, 2023 09:50

ms2008 reviewed Jul 11, 2023

View reviewed changes

bin/kong Show resolved Hide resolved

catbro666 and others added 2 commits July 24, 2023 15:39

Update CHANGELOG.md

782c3e2

Co-authored-by: Qirui(Keery) Nie <[email protected]>

always call prepare_prefix as the prefix directory may not existed and

6c72f0c

the lua_ssl_trusted_certificate config may be updated

ms2008 approved these changes Jul 24, 2023

View reviewed changes

windmgc approved these changes Jul 24, 2023

View reviewed changes

windmgc merged commit 8a1ebba into master Jul 24, 2023

windmgc deleted the bin-kong-to-lua-script branch July 24, 2023 09:51

team-gateway-bot mentioned this pull request Jul 24, 2023

[Backport release/3.4.x] fix(cmd): lack of necessary nginx directives in kong cli nginx.conf #11264

Merged

windmgc mentioned this pull request Jul 25, 2023

fix(cmd): kong vault get doesn't work in dbless mode #10675

Closed

catbro666 added a commit that referenced this pull request Jul 26, 2023

fix(cmd): remove the restriction that some configuration fields are not

7b341d2

allowd to reference vaults introduced by #11127 [FTI-4937](https://konghq.atlassian.net/browse/FTI-4937)

catbro666 mentioned this pull request Jul 26, 2023

fix(cmd): remove the restriction that some configuration fields are not #11291

Merged

kikito mentioned this pull request Aug 9, 2023

chore(*): release 3.4 #11379

Merged

kikito added the backport release/3.3.x label Sep 25, 2023

kikito mentioned this pull request Oct 3, 2023

docs(COPYRIGHT): update copyright info in 3.4.1 #11688

Merged

kikito mentioned this pull request Jan 24, 2024

fix(scripts): fix update-copyright in venv and remove unused repos #12413

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cmd): lack of necessary nginx directives in kong cli nginx.conf #11127

fix(cmd): lack of necessary nginx directives in kong cli nginx.conf #11127

catbro666 commented Jun 27, 2023 •

edited

Loading

catbro666 commented Jul 5, 2023

kikito commented Jul 11, 2023

catbro666 commented Jul 12, 2023 •

edited

Loading

team-gateway-bot commented Sep 25, 2023

kikito commented Sep 25, 2023

fix(cmd): lack of necessary nginx directives in kong cli nginx.conf #11127

fix(cmd): lack of necessary nginx directives in kong cli nginx.conf #11127

Conversation

catbro666 commented Jun 27, 2023 • edited Loading

Summary

Checklist

catbro666 commented Jul 5, 2023

kikito commented Jul 11, 2023

catbro666 commented Jul 12, 2023 • edited Loading

team-gateway-bot commented Sep 25, 2023

kikito commented Sep 25, 2023

catbro666 commented Jun 27, 2023 •

edited

Loading

catbro666 commented Jul 12, 2023 •

edited

Loading