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

catbro666 · 2023-04-14T08:09:01Z

Summary

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 respawning another resty call with the necessary directives (lmdb and lua_ssl stuff) inserted via the --main-conf/--http-conf/--stream-conf options. This PR is trying to make the fix generic so that other commands can reuse this part of logic and we can add other directives conveniently if necessary.

Note we only try this after detecting this specific error no LMDB environment defined in order to avoid infinite loop. And because resty creates a temporary nginx instance so we need to convert the relative paths in the nginx.conf to the absolute path under kong instance prefix.

FTI-4937

Original PR: Kong/kong#10675 [FTI-4937](https://konghq.atlassian.net/browse/FTI-4937)

kong/cmd/vault.lua

VicYP · 2023-04-28T15:28:50Z

@hanshuebner Could you please review this one again? Thanks.

hanshuebner · 2023-04-28T15:47:17Z

@VicYP Not before Wednesday, sorry.

bungle · 2023-05-02T10:24:49Z

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:

patching resty (to include lmdb stuff, and perhaps lua trusted certs)
changing bin/kong to run in shell and then start the resty with required attributes
there is a related fix in ee: https://github.com/Kong/kong-ee/pull/5215 that tries to fix lua trusted certs problem with luasocket that reads system certs (which fixes only that part of this problem)

catbro666 · 2023-05-02T10:36:37Z

Agree. The current fix is a bit hacky. Better to have a more generic way.

catbro666 · 2023-05-04T07:49:02Z

@bungle The tricky part of this problem is that we don't really know what directives to inject before getting the kong conf by calling conf_loader. So we need to run a process first, resty or whatever, to get this information, and then compile this information to obtain the corresponding nginx conf. It feels like the chicken-and-egg question. This limitation seems always exist as long as our cmd implementation still relies on resty. Actually, kong start command is implemented with a similar idea. The different part is kong start calls nginx eventually, while kong vault calls resty again (self-call).

bungle · 2023-05-04T09:41:12Z

Here is a draft of generic fix:
#10785

Co-authored-by: Hans Hübner <[email protected]>

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(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]>

* 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)

windmgc · 2023-07-25T03:00:26Z

close in favor of #11127

* 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)

…s 20 (#10675) KAG-5461

pull-request-size bot added the size/S label Apr 14, 2023

catbro666 changed the title ~~chore(cmd): add a cavert description about cli kong vault get~~ chore(cmd): add a caveat description about cli kong vault get Apr 14, 2023

github-actions bot assigned catbro666 Apr 14, 2023

github-actions bot added the core/cli label Apr 14, 2023

catbro666 force-pushed the FTI-4937-kong-cli-vault-caveats branch from d3645c2 to 396abaa Compare April 14, 2023 08:13

catbro666 added a commit to Kong/docs.konghq.com that referenced this pull request Apr 14, 2023

chore(cmd): add a description of the caveat about kong vault get

d4bf3fb

Original PR: Kong/kong#10675 [FTI-4937](https://konghq.atlassian.net/browse/FTI-4937)

catbro666 mentioned this pull request Apr 14, 2023

chore(cmd): add a description of the caveat about kong vault get Kong/docs.konghq.com#5436

Closed

hanshuebner suggested changes Apr 17, 2023

View reviewed changes

kong/cmd/vault.lua Outdated Show resolved Hide resolved

catbro666 marked this pull request as draft April 18, 2023 10:12

hbagdi added this to the 3.3.0 milestone Apr 18, 2023

catbro666 force-pushed the FTI-4937-kong-cli-vault-caveats branch from 396abaa to dfff1f4 Compare April 26, 2023 03:54

pull-request-size bot added size/L and removed size/S labels Apr 26, 2023

github-actions bot added core/configuration core/templates labels Apr 26, 2023

catbro666 changed the title ~~chore(cmd): add a caveat description about cli kong vault get~~ fix(cmd): kong vault get doesn't work in dbless mode Apr 26, 2023

catbro666 force-pushed the FTI-4937-kong-cli-vault-caveats branch from dfff1f4 to b70f3d2 Compare April 26, 2023 04:43

github-actions bot added the changelog label Apr 26, 2023

catbro666 force-pushed the FTI-4937-kong-cli-vault-caveats branch from b70f3d2 to 41a6882 Compare April 26, 2023 04:45

catbro666 marked this pull request as ready for review April 26, 2023 07:55

catbro666 requested review from hanshuebner and bungle April 26, 2023 09:30

bungle modified the milestones: 3.3.0, 3.4.0 May 2, 2023

catbro666 and others added 11 commits June 26, 2023 14:10

change to absolute path

6c0fde8

fixup

473c2e5

fix path

47f7329

Update CHANGELOG.md

838bd37

Co-authored-by: Hans Hübner <[email protected]>

use return (...) syntax instead

a32111f

don't expose the option and use a better name

9a0c090

pass paths instead of patterns and use better names

f451dda

correctly handle the stdout/stderr/exit code

e388381

preserve original cli args for reusing

a36ddb8

use env variable to terminate recursion

c3d9d69

resty isn't necessarily in the position -1, so add it explicitly

308a3f1

catbro666 force-pushed the FTI-4937-kong-cli-vault-caveats branch from 9cf5de5 to 308a3f1 Compare June 26, 2023 06:11

update the lmdb_map_size to 2048m

634a78d

catbro666 mentioned this pull request Jun 27, 2023

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

Merged

2 tasks

catbro666 marked this pull request as draft July 17, 2023 03:11

catbro666 removed the pr/please review label Jul 17, 2023

catbro666 removed this from the 3.4.0 milestone Jul 17, 2023

windmgc closed this Jul 25, 2023

ms2008 deleted the FTI-4937-kong-cli-vault-caveats branch July 25, 2023 03:21

lhanjian pushed a commit that referenced this pull request Dec 23, 2024

chore(ci): bump slsa-framework/slsa-github-generator to support nodej…

af09268

…s 20 (#10675) KAG-5461

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

catbro666 commented Apr 14, 2023 •

edited

Loading

VicYP commented Apr 28, 2023

hanshuebner commented Apr 28, 2023

bungle commented May 2, 2023

catbro666 commented May 2, 2023 •

edited

Loading

catbro666 commented May 4, 2023 •

edited

Loading

bungle commented May 4, 2023

windmgc commented Jul 25, 2023

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

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

Conversation

catbro666 commented Apr 14, 2023 • edited Loading

Summary

VicYP commented Apr 28, 2023

hanshuebner commented Apr 28, 2023

bungle commented May 2, 2023

catbro666 commented May 2, 2023 • edited Loading

catbro666 commented May 4, 2023 • edited Loading

bungle commented May 4, 2023

windmgc commented Jul 25, 2023

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

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

catbro666 commented Apr 14, 2023 •

edited

Loading

catbro666 commented May 2, 2023 •

edited

Loading

catbro666 commented May 4, 2023 •

edited

Loading