From 29b6d8af16884bb8f0b5cd83a3752980df8548d0 Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Fri, 20 Dec 2024 17:25:29 +0100 Subject: [PATCH] MINOR: hlua: rename "tune.lua.preserve-smp-bool" to "tune.lua.bool-sample-conversion" A better name was found for the option implemented in ec74438 ("MINOR: hlua: add option to preserve bool type from smp to lua") Indeed, "tune.lua.preserve-smp-bool {on | off}" wasn't explicit enough nor did it encourage the adoption of the new "fixed" behavior (vs historical behavior which is now considered as a bug). Thus it becomes "tune.lua.bool-sample-conversion { normal | pre-3.1-bug }" which actively encourage users to switch the new behavior after having patched in-use Lua script if needed. From a technical point of view, the logic remains the same, as the option currently defaults to "pre-3.1-bug" to prevent script breakage, and a warning is emitted if the option isn't set explicily and Lua is used. Documentation and regtests were updated. Must be backported in 3.1 with ec74438 and f2838f5 ("REGTESTS: fix lua-based regtests using tune.lua.smp-preserve-bool") --- doc/configuration.txt | 41 ++++++++++---------- reg-tests/compression/lua_validation.vtc | 2 +- reg-tests/lua/bad_http_clt_req_duration.vtc | 2 +- reg-tests/lua/close_wait_lf.vtc | 2 +- reg-tests/lua/h_txn_get_priv.vtc | 2 +- reg-tests/lua/httpclient_action.vtc | 2 +- reg-tests/lua/lua_httpclient.vtc | 2 +- reg-tests/lua/lua_socket.vtc | 2 +- reg-tests/lua/set_var.vtc | 2 +- reg-tests/lua/txn_get_priv-thread.vtc | 2 +- reg-tests/lua/txn_get_priv.vtc | 2 +- reg-tests/lua/wrong_types_usage.vtc | 2 +- reg-tests/mailers/healthcheckmail.vtc | 2 +- src/hlua.c | 42 ++++++++++----------- 14 files changed, 54 insertions(+), 53 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index d7ad7888b..da10b2509 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -1663,6 +1663,7 @@ The following keywords are supported in the "global" section : - tune.http.maxhdr - tune.idle-pool.shared - tune.idletimer + - tune.lua.bool-sample-conversion - tune.lua.burst-timeout - tune.lua.forced-yield - tune.lua.log.loggers @@ -1670,7 +1671,6 @@ The following keywords are supported in the "global" section : - tune.lua.maxmem - tune.lua.service-timeout - tune.lua.session-timeout - - tune.lua.smp-preserve-bool - tune.lua.task-timeout - tune.max-checks-per-thread - tune.maxaccept @@ -3880,6 +3880,26 @@ tune.listener.multi-queue { on | fair | off } short-lived and it is estimated that the operating system already provides a good enough distribution. The default is "on". +tune.lua.bool-sample-conversion { normal | pre-3.1-bug } + Explicitly tell haproxy how haproxy sample objects should be handled when + pushed to Lua. Indeed, when leveraging native converters, sample fetches or + variables from Lua script (to name a few), haproxy converts the internal + smp type to equivalent Lua type. Because of historical implementation, there + is an ambiguity around boolean handling: when doing Lua -> haproxy smp + conversion, booleans are properly preserved, but when doing haproxy + smp -> Lua conversion, booleans were converted to integers by mistake. This + means that a sample fetch or converter returning a boolean would return an + integer 0 or 1 when leveraged from Lua. Unfortunately, in Lua, booleans and + integers are not interchangeable. Thus, to avoid ambiguities, + "tune.lua.bool-sample-conversion" must explicitly be set to either "normal" + (which means dropping the historical behavior for better consistency) or + "pre-3.1-bug" (enforce historical behavior to prevent existing script logic + from misbehaving). If the option is not set explicitly and a Lua script is + loaded from the configuration, haproxy will emit a warning, and the option + will implicitly default to "pre-3.1-bug" to match with the historical + behavior. It is recommended to set this option to "normal" after ensuring + that in-use Lua scripts are properly handling bool haproxy samples as booleans. + tune.lua.burst-timeout The "burst" execution timeout applies to any Lua handler. If the handler fails to finish or yield before timeout is reached, it will be aborted to @@ -3990,25 +4010,6 @@ tune.lua.task-timeout remain alive during of the lifetime of HAProxy. For example, a task used to check servers. -tune.lua.smp-preserve-bool { on | off } - Explicitly tell haproxy how haproxy sample objects should be handled when - pushed to Lua. Indeed, when using native converters or sample fetches from - Lua script, haproxy converts the smp type to equivalent Lua type. Because - of historical implementation, there is an ambiguity around boolean - handling: when doing Lua -> haproxy smp conversion, booleans are properly - preserved, but when doing haproxy smp -> Lua conversion, booleans were - converted to integers. This means that a sample fetch or converter returning - a boolean would return an integer 0 or 1 when leveraged from Lua. - Unfortunately, in Lua, booleans and integers are not interchangeable. Thus, - to avoid ambiguities, "tune.lua.smp-preserve-bool" must explicitly be set to - either "on" (which means dropping the historical behavior for better - consistency) or "off" (enforce historical behavior to prevent existing script - logic from misbehaving). If the option is not set explicitly and a Lua script - is loaded from the configuration, haproxy will emit a warning, and the option - will implicitly default to "off" to match with the historical behavior. When - possible, it is recommended to set this option to "on" after ensuring that - in-use Lua scripts are properly handling bool haproxy samples as booleans. - tune.max-checks-per-thread Sets the number of active checks per thread above which a thread will actively try to search a less loaded thread to run the health check, or diff --git a/reg-tests/compression/lua_validation.vtc b/reg-tests/compression/lua_validation.vtc index c14d5ca07..2bbf3a1a5 100644 --- a/reg-tests/compression/lua_validation.vtc +++ b/reg-tests/compression/lua_validation.vtc @@ -8,7 +8,7 @@ feature ignore_unknown_macro haproxy h1 -conf { global - tune.lua.smp-preserve-bool on + tune.lua.bool-sample-conversion normal lua-load ${testdir}/lua_validation.lua defaults diff --git a/reg-tests/lua/bad_http_clt_req_duration.vtc b/reg-tests/lua/bad_http_clt_req_duration.vtc index b5d56fff8..ef15982a7 100644 --- a/reg-tests/lua/bad_http_clt_req_duration.vtc +++ b/reg-tests/lua/bad_http_clt_req_duration.vtc @@ -35,7 +35,7 @@ syslog Slog { haproxy h1 -conf { global - tune.lua.smp-preserve-bool on + tune.lua.bool-sample-conversion normal lua-load ${testdir}/bad_http_clt_req_duration.lua defaults diff --git a/reg-tests/lua/close_wait_lf.vtc b/reg-tests/lua/close_wait_lf.vtc index db1a3a9bb..0a2c40d3a 100644 --- a/reg-tests/lua/close_wait_lf.vtc +++ b/reg-tests/lua/close_wait_lf.vtc @@ -30,7 +30,7 @@ haproxy h1 -conf { timeout connect "${HAPROXY_TEST_TIMEOUT-5s}" global - tune.lua.smp-preserve-bool on + tune.lua.bool-sample-conversion normal lua-load ${testdir}/close_wait_lf.lua frontend frt diff --git a/reg-tests/lua/h_txn_get_priv.vtc b/reg-tests/lua/h_txn_get_priv.vtc index e6016259c..8a2931b5b 100644 --- a/reg-tests/lua/h_txn_get_priv.vtc +++ b/reg-tests/lua/h_txn_get_priv.vtc @@ -5,7 +5,7 @@ feature ignore_unknown_macro haproxy h1 -conf { global - tune.lua.smp-preserve-bool on + tune.lua.bool-sample-conversion normal lua-load ${testdir}/h_txn_get_priv.lua defaults diff --git a/reg-tests/lua/httpclient_action.vtc b/reg-tests/lua/httpclient_action.vtc index 05a5d4aee..95baca0d8 100644 --- a/reg-tests/lua/httpclient_action.vtc +++ b/reg-tests/lua/httpclient_action.vtc @@ -12,7 +12,7 @@ feature ignore_unknown_macro haproxy h1 -conf { global - tune.lua.smp-preserve-bool on + tune.lua.bool-sample-conversion normal lua-load ${testdir}/httpclient_action.lua defaults mode tcp diff --git a/reg-tests/lua/lua_httpclient.vtc b/reg-tests/lua/lua_httpclient.vtc index 5ca695305..996d99171 100644 --- a/reg-tests/lua/lua_httpclient.vtc +++ b/reg-tests/lua/lua_httpclient.vtc @@ -38,7 +38,7 @@ server s3 { haproxy h1 -conf { global - tune.lua.smp-preserve-bool on + tune.lua.bool-sample-conversion normal lua-load ${testdir}/lua_httpclient.lua defaults diff --git a/reg-tests/lua/lua_socket.vtc b/reg-tests/lua/lua_socket.vtc index ba8eed093..b3b716ea9 100644 --- a/reg-tests/lua/lua_socket.vtc +++ b/reg-tests/lua/lua_socket.vtc @@ -10,7 +10,7 @@ server s1 { haproxy h1 -conf { global - tune.lua.smp-preserve-bool on + tune.lua.bool-sample-conversion normal lua-load ${testdir}/lua_socket.lua defaults diff --git a/reg-tests/lua/set_var.vtc b/reg-tests/lua/set_var.vtc index 95a854a14..0d018fc4c 100644 --- a/reg-tests/lua/set_var.vtc +++ b/reg-tests/lua/set_var.vtc @@ -11,7 +11,7 @@ haproxy h1 -conf { tune.idle-pool.shared off global - tune.lua.smp-preserve-bool on + tune.lua.bool-sample-conversion normal lua-load ${testdir}/set_var.lua defaults diff --git a/reg-tests/lua/txn_get_priv-thread.vtc b/reg-tests/lua/txn_get_priv-thread.vtc index c4af594fc..3d7ec31b8 100644 --- a/reg-tests/lua/txn_get_priv-thread.vtc +++ b/reg-tests/lua/txn_get_priv-thread.vtc @@ -12,7 +12,7 @@ haproxy h1 -conf { # under us. tune.idle-pool.shared off - tune.lua.smp-preserve-bool on + tune.lua.bool-sample-conversion normal lua-load-per-thread ${testdir}/txn_get_priv.lua lua-load-per-thread ${testdir}/txn_get_priv-print_r.lua diff --git a/reg-tests/lua/txn_get_priv.vtc b/reg-tests/lua/txn_get_priv.vtc index f107e7fd5..2951f110c 100644 --- a/reg-tests/lua/txn_get_priv.vtc +++ b/reg-tests/lua/txn_get_priv.vtc @@ -6,7 +6,7 @@ feature ignore_unknown_macro haproxy h1 -conf { global - tune.lua.smp-preserve-bool on + tune.lua.bool-sample-conversion normal lua-load ${testdir}/txn_get_priv.lua lua-load ${testdir}/txn_get_priv-print_r.lua diff --git a/reg-tests/lua/wrong_types_usage.vtc b/reg-tests/lua/wrong_types_usage.vtc index c28d45ef9..f06b18a03 100644 --- a/reg-tests/lua/wrong_types_usage.vtc +++ b/reg-tests/lua/wrong_types_usage.vtc @@ -43,7 +43,7 @@ server s1 -repeat 2 { haproxy h1 -conf { global - tune.lua.smp-preserve-bool on + tune.lua.bool-sample-conversion normal lua-load ${testdir}/wrong_types_usage.lua defaults diff --git a/reg-tests/mailers/healthcheckmail.vtc b/reg-tests/mailers/healthcheckmail.vtc index 20f4ccb6c..dbf4e9cc6 100644 --- a/reg-tests/mailers/healthcheckmail.vtc +++ b/reg-tests/mailers/healthcheckmail.vtc @@ -12,7 +12,7 @@ syslog S1 -level notice { haproxy h1 -conf { global - tune.lua.smp-preserve-bool on + tune.lua.bool-sample-conversion normal lua-load ${testdir}/mailers.lua lua-load ${testdir}/healthcheckmail.lua diff --git a/src/hlua.c b/src/hlua.c index 4889b6776..f3ed8999a 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -84,17 +84,17 @@ enum hlua_log_opt { /* default log options, made of flags in hlua_log_opt */ static uint hlua_log_opts = HLUA_LOG_LOGGERS_ON | HLUA_LOG_STDERR_AUTO; -#define HLUA_SMP_PRESERVE_BOOL_UNK 0x0 -#define HLUA_SMP_PRESERVE_BOOL_ON 0x1 -#define HLUA_SMP_PRESERVE_BOOL_OFF 0X2 +#define HLUA_BOOL_SAMPLE_CONVERSION_UNK 0x0 +#define HLUA_BOOL_SAMPLE_CONVERSION_NORMAL 0x1 +#define HLUA_BOOL_SAMPLE_CONVERSION_BUG 0X2 /* used to know how to deal with smp2lua bool handling, option implicitly - * defaults to historical behavior (off), but it has to be set explicitly + * defaults to historical behavior (BUG), but it has to be set explicitly * to avoid ambiguity, else a warning will be emitted * - * FIXME: make it default to ON in 3.3?? + * FIXME: make it default to NORMAL in 3.3?? */ -static uint8_t hlua_smp_preserve_bool = HLUA_SMP_PRESERVE_BOOL_UNK; +static uint8_t hlua_bool_sample_conversion = HLUA_BOOL_SAMPLE_CONVERSION_UNK; /* Lua uses longjmp to perform yield or throwing errors. This * macro is used only for identifying the function that can @@ -1100,7 +1100,7 @@ __LJMP static int hlua_smp2lua(lua_State *L, struct sample *smp) lua_pushinteger(L, smp->data.u.sint); break; case SMP_T_BOOL: - if (hlua_smp_preserve_bool == HLUA_SMP_PRESERVE_BOOL_ON) + if (hlua_bool_sample_conversion == HLUA_BOOL_SAMPLE_CONVERSION_NORMAL) lua_pushboolean(L, !!smp->data.u.sint); else lua_pushinteger(L, smp->data.u.sint); @@ -12953,19 +12953,19 @@ static int hlua_cfg_parse_log_stderr(char **args, int section_type, struct proxy return 0; } -static int hlua_cfg_parse_smp_preserve_bool(char **args, int section_type, struct proxy *curpx, - const struct proxy *defpx, const char *file, int line, - char **err) +static int hlua_cfg_parse_bool_sample_conversion(char **args, int section_type, struct proxy *curpx, + const struct proxy *defpx, const char *file, int line, + char **err) { if (too_many_args(1, args, err, NULL)) return -1; - if (strcmp(args[1], "on") == 0) - hlua_smp_preserve_bool = HLUA_SMP_PRESERVE_BOOL_ON; - else if (strcmp(args[1], "off") == 0) - hlua_smp_preserve_bool = HLUA_SMP_PRESERVE_BOOL_OFF; + if (strcmp(args[1], "normal") == 0) + hlua_bool_sample_conversion = HLUA_BOOL_SAMPLE_CONVERSION_NORMAL; + else if (strcmp(args[1], "pre-3.1-bug") == 0) + hlua_bool_sample_conversion = HLUA_BOOL_SAMPLE_CONVERSION_BUG; else { - memprintf(err, "'%s' expects either 'on' or 'off' but got '%s'.", args[0], args[1]); + memprintf(err, "'%s' expects either 'normal' or 'pre-3.1-bug' but got '%s'.", args[0], args[1]); return -1; } return 0; @@ -12994,14 +12994,14 @@ static int hlua_load_state(char **args, lua_State *L, char **err) /* we know we get there if "lua-load" or "lua-load-per-thread" was * used in the config */ - if (hlua_smp_preserve_bool == HLUA_SMP_PRESERVE_BOOL_UNK) { - /* hlua_smp_preserve_bool tunable must be explicitly set to + if (hlua_bool_sample_conversion == HLUA_BOOL_SAMPLE_CONVERSION_UNK) { + /* hlua_bool_sample_conversion tunable must be explicitly set to * avoid ambiguity, so we raise a warning (but only if lua * is actually used */ - ha_warning("hlua: please set \"tune.lua.smp-preserve-bool\" tunable to " - "either \"on\" or \"off\" explicitly to avoid ambiguities. " - "Defaulting to \"off\".\n"); + ha_warning("hlua: please set \"tune.lua.bool-sample-conversion\" tunable " + "to either \"normal\" or \"pre-3.1-bug\" explicitly to avoid " + "ambiguities. Defaulting to \"pre-3.1-bug\".\n"); } } @@ -13236,7 +13236,7 @@ static struct cfg_kw_list cfg_kws = {{ },{ { CFG_GLOBAL, "tune.lua.maxmem", hlua_parse_maxmem }, { CFG_GLOBAL, "tune.lua.log.loggers", hlua_cfg_parse_log_loggers }, { CFG_GLOBAL, "tune.lua.log.stderr", hlua_cfg_parse_log_stderr }, - { CFG_GLOBAL, "tune.lua.smp-preserve-bool", hlua_cfg_parse_smp_preserve_bool }, + { CFG_GLOBAL, "tune.lua.bool-sample-conversion", hlua_cfg_parse_bool_sample_conversion }, { 0, NULL, NULL }, }};