kong icon indicating copy to clipboard operation
kong copied to clipboard

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

Open catbro666 opened this issue 2 years ago • 8 comments

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

catbro666 avatar Apr 14 '23 08:04 catbro666

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

VicYP avatar Apr 28 '23 15:04 VicYP

@VicYP Not before Wednesday, sorry.

hanshuebner avatar Apr 28 '23 15:04 hanshuebner

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:

  1. patching resty (to include lmdb stuff, and perhaps lua trusted certs)
  2. changing bin/kong to run in shell and then start the resty with required attributes
  3. 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)

bungle avatar May 02 '23 10:05 bungle

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

catbro666 avatar May 02 '23 10:05 catbro666

@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).

catbro666 avatar May 04 '23 07:05 catbro666

Here is a draft of generic fix: https://github.com/Kong/kong/pull/10785

bungle avatar May 04 '23 09:05 bungle

Updated this PR to make it more generic. The basic idea keeps the same, but the main processing logic is moved from kong/cmd/vault.lua forward to kong/cmd/init.lua so that other commands can reuse this part of logic. Both lmdb stuff and lua_ssl stuff are considered in this PR and when necessary we can add other directives conveniently.

~~TODO: add more tests.~~

catbro666 avatar May 05 '23 11:05 catbro666

Because of the reason mentioned above, I don't think statically adding directives into resty_cli will work. The config can be specified by env or custom config file or inherit from the default configuration. And some preparation like .ca_combined may also be required. So it's still most convenient and maintainable to load and compile the conf in Lua land. @bungle What do you think?

catbro666 avatar May 08 '23 13:05 catbro666

close in favor of #11127

windmgc avatar Jul 25 '23 03:07 windmgc