[patches] [cfg 1/1] cfg: Add constraints on box.cfg params

imarkov imarkov at tarantool.org
Tue Feb 20 13:52:51 MSK 2018


From: IlyaMarkovMipt <markovilya197 at gmail.com>

* Introduce limitations on combinations of box.cfg parameters
* Add restriction on log type file and log_nonblock=true
* Add restriction on log type syslog and log_format json
* Change default value of log_nonblock from true to false

Relates #3014 #3072
---
 src/box/lua/load_cfg.lua        | 53 ++++++++++++++++++++++++++++++++++++++++-
 test/app-tap/init_script.result |  2 +-
 test/app-tap/logger.test.lua    |  5 +---
 test/box-tap/cfg.test.lua       | 16 +++++++++----
 test/box/admin.result           |  2 +-
 test/box/cfg.result             |  4 ++--
 6 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 4ac0408..20f7e65 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -30,7 +30,7 @@ local default_cfg = {
     vinyl_page_size           = 8 * 1024,
     vinyl_bloom_fpr           = 0.05,
     log                 = nil,
-    log_nonblock        = true,
+    log_nonblock        = false,
     log_level           = 5,
     log_format          = "plain",
     io_collect_interval = nil,
@@ -343,6 +343,28 @@ local box_cfg_guard_whitelist = {
     NULL = true;
 };
 
+local logger_types = {
+    LOGGER_FILE = 1,
+    LOGGER_PIPE = 2,
+    LOGGER_SYSLOG = 3
+}
+
+local function parse_logger_type(log)
+    if log == nil then
+        return nil
+    end
+    if log:match("^|") or log:match("^pipe:") then
+        return logger_types.LOGGER_PIPE
+    elseif log:match("^syslog:") then
+        return logger_types.LOGGER_SYSLOG
+    elseif log:match("^file:") or not log:match("^:") then
+        return logger_types.LOGGER_FILE
+    else
+        return box.error(box.error.ILLEGAl_PARAMS,
+        "expecting a file name or a prefix, such as '|', 'pipe:', 'syslog:'")
+    end
+end
+
 local box = require('box')
 -- Move all box members except 'error' to box_configured
 local box_configured = {}
@@ -360,6 +382,34 @@ setmetatable(box, {
      end
 })
 
+-- List of combinations that are prohibited in cfg
+-- Each combination consists of list of parameters descriptions
+-- Each parameter description includes parameter name, its value and
+-- optionally function that converts box.cfg option to comparable value
+local box_cfg_contrary_combinations = {
+    {{"log_format", "json"}, {"log", logger_types.LOGGER_SYSLOG, parse_logger_type}},
+    {{"log_nonblock", true}, {"log", logger_types.LOGGER_FILE, parse_logger_type}}
+}
+
+local function verify_combinations(contrary_combinations)
+    for _, combination in pairs(contrary_combinations) do
+        local params = {}
+        for _, parameter in pairs(combination) do
+            local value = box.cfg[parameter[1]]
+            if parameter[3] ~= nil then
+                value = parameter[3](value)
+            end
+            if value ~= parameter[2] then
+                goto not_match
+            end
+            table.insert(params, parameter[1])
+        end
+        box.error(box.error.ILLEGAL_PARAMS, "wrong combination of " ..
+                    table.concat(params, ", "))
+        ::not_match::
+    end
+end
+
 local function load_cfg(cfg)
     box.internal.schema.init()
     cfg = upgrade_cfg(cfg, translate_cfg)
@@ -394,6 +444,7 @@ local function load_cfg(cfg)
             end
         end
     end
+    verify_combinations(box_cfg_contrary_combinations)
     if not box.cfg.read_only and not box.cfg.replication then
         box.schema.upgrade{auto = true}
     end
diff --git a/test/app-tap/init_script.result b/test/app-tap/init_script.result
index 53f87a5..65d3dec 100644
--- a/test/app-tap/init_script.result
+++ b/test/app-tap/init_script.result
@@ -13,7 +13,7 @@ box.cfg
 8	log:tarantool.log
 9	log_format:plain
 10	log_level:5
-11	log_nonblock:true
+11	log_nonblock:false
 12	memtx_dir:.
 13	memtx_max_tuple_size:1048576
 14	memtx_memory:107374182
diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
index 2d0f333..61488ee 100755
--- a/test/app-tap/logger.test.lua
+++ b/test/app-tap/logger.test.lua
@@ -1,7 +1,7 @@
 #!/usr/bin/env tarantool
 
 local test = require('tap').test('log')
-test:plan(22)
+test:plan(21)
 
 --
 -- Check that Tarantool creates ADMIN session for #! script
@@ -59,9 +59,6 @@ debug = require('debug')
 
 test:ok(log.info(true) == nil, 'check tarantool crash (gh-2516)')
 
-s, err = pcall(box.cfg, {log_format='json', log="syslog:identity:tarantool"})
-test:ok(not s, "check json not in syslog")
-
 box.cfg{log=filename,
     memtx_memory=107374182,
     log_format = "json"}
diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua
index 67991ec..2465688 100755
--- a/test/box-tap/cfg.test.lua
+++ b/test/box-tap/cfg.test.lua
@@ -6,7 +6,7 @@ local socket = require('socket')
 local fio = require('fio')
 local uuid = require('uuid')
 local msgpack = require('msgpack')
-test:plan(80)
+test:plan(82)
 
 --------------------------------------------------------------------------------
 -- Invalid values
@@ -34,6 +34,14 @@ invalid('log', ':')
 invalid('log', 'syslog:xxx=')
 invalid('log_level', 'unknown')
 
+local function invalid_combinations(name, val)
+    local status, result = pcall(box.cfg, {[name]=val})
+    test:ok(not status and result:match('Incorrect'), 'invalid '..name)
+end
+
+invalid_combinations("log, log_nonblock", {log = "1.log", log_nonblock = true})
+invalid_combinations("log, log_format", {log = "syslog:identity:tarantool", log_format = 'json'})
+
 test:is(type(box.cfg), 'function', 'box is not started')
 
 --------------------------------------------------------------------------------
@@ -177,10 +185,10 @@ test:is(run_script(code), PANIC, 'snap_dir is invalid')
 code = [[ box.cfg{ wal_dir='invalid' } ]]
 test:is(run_script(code), PANIC, 'wal_dir is invalid')
 
-test:is(box.cfg.log_nonblock, true, "log_nonblock default value")
+test:is(box.cfg.log_nonblock, false, "log_nonblock default value")
 code = [[
-box.cfg{log_nonblock = false }
-os.exit(box.cfg.log_nonblock == false and 0 or 1)
+box.cfg{log_nonblock = true }
+os.exit(box.cfg.log_nonblock == true and 0 or 1)
 ]]
 test:is(run_script(code), 0, "log_nonblock new value")
 
diff --git a/test/box/admin.result b/test/box/admin.result
index 13e599e..eee3d5e 100644
--- a/test/box/admin.result
+++ b/test/box/admin.result
@@ -39,7 +39,7 @@ cfg_filter(box.cfg)
   - - log_level
     - 5
   - - log_nonblock
-    - true
+    - false
   - - memtx_dir
     - <hidden>
   - - memtx_max_tuple_size
diff --git a/test/box/cfg.result b/test/box/cfg.result
index 9f0ad59..9844495 100644
--- a/test/box/cfg.result
+++ b/test/box/cfg.result
@@ -35,7 +35,7 @@ cfg_filter(box.cfg)
   - - log_level
     - 5
   - - log_nonblock
-    - true
+    - false
   - - memtx_dir
     - <hidden>
   - - memtx_max_tuple_size
@@ -122,7 +122,7 @@ cfg_filter(box.cfg)
   - - log_level
     - 5
   - - log_nonblock
-    - true
+    - false
   - - memtx_dir
     - <hidden>
   - - memtx_max_tuple_size
-- 
2.7.4




More information about the Tarantool-patches mailing list