From dde7342c52078b3777859b0abfacdcb3970e2c1b Mon Sep 17 00:00:00 2001
From: Alexander Turenko <alexander.turenko@tarantool.org>
Date: Sat, 23 Dec 2023 03:58:40 +0300
Subject: [PATCH] box: support TT_* uri env vars with query params
`TT_LISTEN` and `TT_REPLICATION` environment variables were interpreted
by `box.cfg()` in a confusing way if query parameters with values are
present. For example, `localhost:3301?transport=plain` was interpreted
as the following map: `{['localhost:3301?transport'] = 'plain'}`. Later,
`box.cfg()` looks into this map for known URI fields like `login`,
`password`, `uri`, `host`, `service` and so on. It found nothing and
doesn't start a listening socket.
The reason of such a behaviour is that the environment value is
interpreted as a mapping in the `key=value,key=value` format, because
there is `=` in it.
The patch changes this behavior for an `key=value,key=value` environment
variable that contains `?` in a key: now such a value is not interpreted
as a mapping.
Note: Everything said above is also applicable to the so called
multilisten case: when several URIs are defined in the environment
variable. The following URI list is interpreted correctly now.
NOWRAP
```sh
export TT_LISTEN=localhost:3301?transport=plain,localhost:3302?transport=plain
```
NOWRAP
Note 2: Examples are given with the `plain` transport, which is default,
but the query parameters are the way to define TLS options. They're
supported in Tarantool Enterprise Edition, see [1].
Fixes #9539
NO_DOC=bugfix
[1]: https://www.tarantool.io/en/doc/latest/enterprise/security/#traffic-encryption
---
...9539-box-cfg-env-vars-with-query-params.md | 4 +
src/box/lua/load_cfg.lua | 45 +++-
test/box-luatest/box_cfg_env_test.lua | 255 ++++++++++++++++++
3 files changed, 295 insertions(+), 9 deletions(-)
create mode 100644 changelogs/unreleased/gh-9539-box-cfg-env-vars-with-query-params.md
create mode 100644 test/box-luatest/box_cfg_env_test.lua
diff --git a/changelogs/unreleased/gh-9539-box-cfg-env-vars-with-query-params.md b/changelogs/unreleased/gh-9539-box-cfg-env-vars-with-query-params.md
new file mode 100644
index 0000000000..59fe7ceb4d
--- /dev/null
+++ b/changelogs/unreleased/gh-9539-box-cfg-env-vars-with-query-params.md
@@ -0,0 +1,4 @@
+## bugfix/box
+
+* Now `box.cfg()` correctly interprets the `TT_LISTEN` and `TT_REPLICATION`
+ environment variables with query parameters (gh-9539).
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index fd678e918a..1a5fe6e2e4 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -1209,9 +1209,12 @@ local function get_option_from_env(option)
if param_type:find('table') and (raw_value:startswith('{') or
raw_value:startswith('[')) then
return json.decode(raw_value)
- elseif param_type:find('table') and raw_value:find('=') then
+ end
+
+ if param_type:find('table') and raw_value:find('=') then
assert(not param_type:find('boolean'))
local res = {}
+ local contains_uri = false
for _, v in ipairs(raw_value:split(',')) do
local eq = v:find('=')
if eq == nil then
@@ -1226,17 +1229,37 @@ local function get_option_from_env(option)
'in `key=value` or `value` format, ' ..
'`key` must not be empty'))
end
+ -- Don't interpret `=` as a key-value separator if
+ -- there is `?` in a key.
+ --
+ -- Otherwise, for example,
+ -- `localhost:3301?transport=plain` would be
+ -- interpreted as the following map.
+ --
+ -- {
+ -- ['localhost:3301?transport'] = 'plain',
+ -- }
+ if lhs:find('?') then
+ contains_uri = true
+ end
res[lhs] = tonumber(rhs) or rhs
end
- return res
- elseif param_type:find('table') and raw_value:find(',') then
+ if not contains_uri then
+ return res
+ end
+ -- Fall through otherwise.
+ end
+
+ if param_type:find('table') and raw_value:find(',') then
assert(not param_type:find('boolean'))
local res = {}
for i, v in ipairs(raw_value:split(',')) do
res[i] = tonumber(v) or v
end
return res
- elseif param_type:find('boolean') then
+ end
+
+ if param_type:find('boolean') then
assert(param_type == 'boolean')
if raw_value:lower() == 'false' then
return false
@@ -1244,20 +1267,24 @@ local function get_option_from_env(option)
return true
end
error(err_msg_fmt:format(env_var_name, option, '"true" or "false"'))
- elseif param_type == 'number' then
+ end
+
+ if param_type == 'number' then
local res = tonumber(raw_value)
if res == nil then
error(err_msg_fmt:format(env_var_name, option,
'convertible to a number'))
end
return res
- elseif param_type:find('number') then
+ end
+
+ if param_type:find('number') then
assert(not param_type:find('boolean'))
return tonumber(raw_value) or raw_value
- else
- assert(param_type == 'string')
- return raw_value
end
+
+ assert(param_type == 'string')
+ return raw_value
end
-- Get options from env vars for given set.
diff --git a/test/box-luatest/box_cfg_env_test.lua b/test/box-luatest/box_cfg_env_test.lua
new file mode 100644
index 0000000000..57f7af9375
--- /dev/null
+++ b/test/box-luatest/box_cfg_env_test.lua
@@ -0,0 +1,255 @@
+local fun = require('fun')
+local json = require('json')
+local t = require('luatest')
+local treegen = require('test.treegen')
+local justrun = require('test.justrun')
+
+local g = t.group()
+
+g.before_all(treegen.init)
+g.after_all(treegen.clean)
+
+-- TT_LISTEN and TT_REPLICATION have many allowed forms.
+--
+-- We can't listen on hardcoded port numbers in the test, because
+-- we don't know whether it will be free on running of the test
+-- suite. Attempt to do so would ruin the stability of the test.
+--
+-- So, these test cases don't call `box.cfg()`, but rather use an
+-- internal API to parse environment variables into box.cfg option
+-- values.
+local uri_list_cases = {
+ -- Plain URI.
+ {
+ env = '3301',
+ cfg = 3301,
+ },
+ {
+ env = 'localhost:3301',
+ cfg = 'localhost:3301',
+ },
+ {
+ env = 'localhost:3301?transport=plain',
+ cfg = 'localhost:3301?transport=plain',
+ },
+ -- Array of plain URIs.
+ {
+ env = '3301,3302',
+ cfg = {3301, 3302},
+ },
+ {
+ env = 'localhost:3301,localhost:3302',
+ cfg = {'localhost:3301', 'localhost:3302'},
+ },
+ {
+ env = 'localhost:3301?transport=plain,' ..
+ 'localhost:3302?transport=plain',
+ cfg = {'localhost:3301?transport=plain',
+ 'localhost:3302?transport=plain'},
+ },
+ -- Mapping.
+ {
+ env = 'uri=3301',
+ cfg = {uri = 3301},
+ },
+ {
+ env = 'uri=localhost:3301',
+ cfg = {uri = 'localhost:3301'},
+ },
+ {
+ env = 'login=foo,password=bar,uri=localhost:3301',
+ cfg = {login = 'foo', password = 'bar', uri = 'localhost:3301'},
+ },
+ -- JSON object.
+ {
+ env = '{"uri": "3301"}',
+ cfg = {uri = '3301'},
+ },
+ {
+ env = '{"uri": "localhost:3301"}',
+ cfg = {uri = 'localhost:3301'},
+ },
+ {
+ env = '{"uri": "localhost:3301", "params": {"transport": "plain"}}',
+ cfg = {uri = 'localhost:3301', params = {transport = 'plain'}},
+ },
+ -- JSON array of numbers.
+ {
+ env = '[3301, 3302]',
+ cfg = {3301, 3302},
+ },
+ -- JSON array of strings.
+ {
+ env = '["3301", "3302"]',
+ cfg = {'3301', '3302'},
+ },
+ {
+ env = '["localhost:3301", "localhost:3302"]',
+ cfg = {'localhost:3301', 'localhost:3302'},
+ },
+ {
+ env = '["localhost:3301?transport=plain", ' ..
+ '"localhost:3302?transport=plain"]',
+ cfg = {'localhost:3301?transport=plain',
+ 'localhost:3302?transport=plain'},
+ },
+ -- JSON array of objects.
+ {
+ env = '[{"uri": "3301"}, {"uri": "3302"}]',
+ cfg = {
+ {uri = '3301'},
+ {uri = '3302'},
+ },
+ },
+ {
+ env = '[{"uri": "localhost:3301"}, {"uri": "localhost:3302"}]',
+ cfg = {
+ {uri = 'localhost:3301'},
+ {uri = 'localhost:3302'},
+ },
+ },
+ {
+ env = '[{"uri": "localhost:3301", "params": {"transport": "plain"}},' ..
+ '{"uri": "localhost:3302", "params": {"transport": "plain"}}]',
+ cfg = {
+ {uri = 'localhost:3301', params = {transport = 'plain'}},
+ {uri = 'localhost:3302', params = {transport = 'plain'}},
+ },
+ },
+}
+
+-- Verify TT_LISTEN and TT_REPLICATION using box.internal.cfg.env.
+--
+-- It is a kind of a unit test.
+--
+-- All the test cases are run using one popen call
+-- (justrun.tarantool()) that speeds up the execution.
+g.test_uri_list = function(g)
+ -- Write cases.lua and main.lua.
+ local dir = treegen.prepare_directory(g, {}, {})
+ treegen.write_script(dir, 'cases.lua', json.encode(uri_list_cases))
+ treegen.write_script(dir, 'main.lua', string.dump(function()
+ local json = require('json')
+ local fio = require('fio')
+ local t = require('luatest')
+
+ -- Read cases.lua.
+ local fh, err = fio.open('cases.lua', {'O_RDONLY'})
+ if err ~= nil then
+ error(err)
+ end
+ local cases_str, err = fh:read()
+ if err ~= nil then
+ error(err)
+ end
+ fh:close()
+ local cases = json.decode(cases_str)
+
+ local function verify(case, env_var_name, box_cfg_option_name)
+ os.setenv(env_var_name, case.env)
+ local res = box.internal.cfg.env[box_cfg_option_name]
+ local ok, err = pcall(t.assert_equals, res, case.cfg)
+ if not ok then
+ error(err.message, 0)
+ end
+ os.setenv(env_var_name, nil)
+ end
+
+ -- Set the environment variable, get the box.cfg option
+ -- from box.internal.cfg.env, check it against a reference
+ -- value, unset the environment variable.
+ for _, case in ipairs(cases) do
+ verify(case, 'TT_LISTEN', 'listen')
+ verify(case, 'TT_REPLICATION', 'replication')
+ end
+ end))
+
+ local opts = {nojson = true, stderr = true}
+ local res = justrun.tarantool(dir, {}, {'main.lua'}, opts)
+ t.assert_equals(res, {
+ exit_code = 0,
+ stdout = '',
+ stderr = '',
+ })
+end
+
+-- These test cases use a real box.cfg() call inside a child
+-- process. A kind of a functional test.
+local cases = {
+ -- String options.
+ {
+ env = {['TT_LOG_FORMAT'] = 'json'},
+ cfg = {log_format = 'json'},
+ },
+ {
+ env = {['TT_LOG_LEVEL'] = 'debug'},
+ cfg = {log_level = 'debug'},
+ },
+ {
+ env = {['TT_CUSTOM_PROC_TITLE'] = 'mybox'},
+ cfg = {custom_proc_title = 'mybox'},
+ },
+ -- Numeric options.
+ {
+ env = {['TT_READAHEAD'] = '32640'},
+ cfg = {readahead = 32640},
+ },
+ {
+ env = {['TT_LOG_LEVEL'] = '7'},
+ cfg = {log_level = 7},
+ },
+ {
+ env = {['TT_REPLICATION_TIMEOUT'] = '0.3'},
+ cfg = {replication_timeout = 0.3},
+ },
+ {
+ env = {['TT_IPROTO_THREADS'] = '16'},
+ cfg = {iproto_threads = 16},
+ },
+ -- Boolean options.
+ {
+ env = {['TT_MEMTX_USE_MVCC_ENGINE'] = 'true'},
+ cfg = {memtx_use_mvcc_engine = true},
+ },
+ {
+ env = {['TT_MEMTX_USE_MVCC_ENGINE'] = 'false'},
+ cfg = {memtx_use_mvcc_engine = false},
+ },
+ {
+ env = {['TT_STRIP_CORE'] = 'false'},
+ cfg = {strip_core = false},
+ },
+}
+
+-- Write a script to be used in the test cases below.
+g.before_all(function(g)
+ g.dir = treegen.prepare_directory(g, {}, {})
+ treegen.write_script(g.dir, 'main.lua', [[
+ local json = require('json')
+ box.cfg()
+ print(json.encode(box.cfg))
+ os.exit()
+ ]])
+end)
+
+-- Verify TT_* variables of different types.
+for i, case in ipairs(cases) do
+ g[('test_box_cfg_%03d'):format(i)] = function(g)
+ local res = justrun.tarantool(g.dir, case.env, {'main.lua'})
+
+ local res_cfg = fun.iter(case.cfg):map(function(box_cfg_option_name)
+ if res.stdout == nil or #res.stdout ~= 1 then
+ return box_cfg_option_name, '???'
+ end
+ return box_cfg_option_name, res.stdout[1][box_cfg_option_name]
+ end):tomap()
+
+ t.assert_equals({
+ exit_code = res.exit_code,
+ cfg = res_cfg,
+ }, {
+ exit_code = 0,
+ cfg = case.cfg,
+ })
+ end
+end
--
GitLab