From 1617204524344172f0be4a57f5e77f80d2a4723d Mon Sep 17 00:00:00 2001 From: wei Date: Mon, 6 Jul 2026 23:43:44 +0800 Subject: [PATCH 1/3] fix(mysql): fail update-parameter.sh on real SET GLOBAL errors Previously only ERROR 1045 failed the reconfigure exec action; every other error (unknown variable, invalid value, connection failure) was silently ignored and the Ops reported success while the running value never changed. Now tolerate exactly the two legitimate cannot-apply- online cases with explicit logs (1238 read-only -> effective after restart via rendered my.cnf; 1193 on loose_-prefixed params -> plugin not loaded), and exit non-zero on everything else. loose_ prefix strip is now anchored to the prefix instead of a global substitution. Fixes #3079 Co-Authored-By: Claude Fable 5 --- addons/mysql/scripts/update-parameter.sh | 31 ++++++++++++++++++------ 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/addons/mysql/scripts/update-parameter.sh b/addons/mysql/scripts/update-parameter.sh index 0f4cea1249..0fd5db40bc 100644 --- a/addons/mysql/scripts/update-parameter.sh +++ b/addons/mysql/scripts/update-parameter.sh @@ -8,8 +8,12 @@ function mysql_exec() { paramName="${1:?missing param name}" paramValue="${2:?missing value}" +# loose_-prefixed parameters keep MySQL's loose semantics: absence of the +# variable (plugin not loaded) is tolerated, any other failure is not. +is_loose=false if echo "${paramName}" | grep -q "^loose_"; then - paramName=${paramName//"loose_"/} + paramName=${paramName#loose_} + is_loose=true fi paramName=$(echo "${paramName}" | tr '-' '_') @@ -39,12 +43,25 @@ else fi if [ $status -ne 0 ]; then - if echo "${ret}" | grep -q "ERROR 1045 (28000)"; then - echo "Failed to set parameter ${paramName} to value ${paramValue}, result: ${ret}" - exit 1 + # Reconfigure must not judge success by this single SET GLOBAL alone: + # some parameters cannot be applied online and legitimately land here. + # Tolerate exactly those, loudly; fail everything else so the Ops does + # not report success for a value that was never applied. + if echo "${ret}" | grep -q "ERROR 1238 "; then + # Read-only variable: it cannot change online. The framework has + # already persisted it into the rendered my.cnf, so it takes effect + # on the next restart. + echo "Parameter ${paramName} is read-only (ERROR 1238); it will take effect after the next restart." + exit 0 fi - # Ignore other errors -else - echo "Set parameter ${paramName} to value ${paramValue}, result: ${ret}" + if [ "${is_loose}" = "true" ] && echo "${ret}" | grep -q "ERROR 1193 "; then + # loose_ parameter whose plugin is not loaded: absence is tolerated + # by MySQL's loose semantics, mirror that here. + echo "Parameter loose_${paramName} is unknown on this instance (plugin not loaded); skipped." + exit 0 + fi + echo "Failed to set parameter ${paramName} to value ${paramValue}, result: ${ret}" >&2 + exit 1 fi +echo "Set parameter ${paramName} to value ${paramValue}" From 7a9820b923a23ebcf6bcdb7e9c71f8004f9ca6b1 Mon Sep 17 00:00:00 2001 From: wei Date: Tue, 7 Jul 2026 01:56:59 +0800 Subject: [PATCH 2/3] test(mysql): shellspec coverage for update-parameter.sh error contract 10 cases: numeric/suffix/quoting/dash-normalization apply paths with query capture, tolerated 1238 and loose_+1193 paths, and hard-failure paths (bare 1193, 1231, 1045, 2003). Co-Authored-By: Claude Fable 5 --- .../scripts-ut-spec/update_parameter_spec.sh | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 addons/mysql/scripts-ut-spec/update_parameter_spec.sh diff --git a/addons/mysql/scripts-ut-spec/update_parameter_spec.sh b/addons/mysql/scripts-ut-spec/update_parameter_spec.sh new file mode 100644 index 0000000000..99d4a849d6 --- /dev/null +++ b/addons/mysql/scripts-ut-spec/update_parameter_spec.sh @@ -0,0 +1,117 @@ +# shellcheck shell=bash +# shellcheck disable=SC2034 + +Describe "MySQL update-parameter Script Tests" + + setup() { + MYSQL_ADMIN_USER="kbadmin" + MYSQL_ADMIN_PASSWORD="secret" + MOCK_QUERY_FILE=$(mktemp) + } + cleanup() { + rm -f "${MOCK_QUERY_FILE}" + } + BeforeEach "setup" + AfterEach "cleanup" + + Describe "success path" + mysql() { + # last argument is the query + eval "printf '%s' \"\${$#}\"" > "${MOCK_QUERY_FILE}" + return 0 + } + + It "applies a plain numeric value and reports success" + When run source ../scripts/update-parameter.sh max_connections 500 + The status should be success + The stdout should include "Set parameter max_connections to value 500" + The contents of file "${MOCK_QUERY_FILE}" should equal "SET GLOBAL max_connections = 500;" + End + + It "converts size suffixes to bytes" + When run source ../scripts/update-parameter.sh innodb_buffer_pool_size 128M + The status should be success + The stdout should include "Set parameter" + The contents of file "${MOCK_QUERY_FILE}" should equal "SET GLOBAL innodb_buffer_pool_size = 134217728;" + End + + It "quotes non-numeric values" + When run source ../scripts/update-parameter.sh sql_mode NO_ENGINE_SUBSTITUTION + The status should be success + The stdout should include "Set parameter" + The contents of file "${MOCK_QUERY_FILE}" should equal "SET GLOBAL sql_mode = 'NO_ENGINE_SUBSTITUTION';" + End + + It "normalizes dashes to underscores" + When run source ../scripts/update-parameter.sh max-connections 500 + The status should be success + The stdout should include "Set parameter max_connections" + The contents of file "${MOCK_QUERY_FILE}" should equal "SET GLOBAL max_connections = 500;" + End + End + + Describe "tolerated cannot-apply-online errors" + It "tolerates ERROR 1238 (read-only variable) and says it applies after restart" + mysql() { + echo "ERROR 1238 (HY000) at line 1: Variable 'gtid_mode' is a read-only variable" >&2 + return 1 + } + When run source ../scripts/update-parameter.sh gtid_mode ON + The status should be success + The stdout should include "read-only" + The stdout should include "after the next restart" + End + + It "tolerates ERROR 1193 for loose_-prefixed parameters (plugin not loaded)" + mysql() { + echo "ERROR 1193 (HY000): Unknown system variable 'audit_log_policy'" >&2 + return 1 + } + When run source ../scripts/update-parameter.sh loose_audit_log_policy ALL + The status should be success + The stdout should include "skipped" + End + End + + Describe "real errors fail the action" + It "fails on ERROR 1193 for a parameter without the loose_ prefix" + mysql() { + echo "ERROR 1193 (HY000): Unknown system variable 'no_such_variable'" >&2 + return 1 + } + When run source ../scripts/update-parameter.sh no_such_variable 1 + The status should be failure + The stderr should include "Failed to set parameter no_such_variable" + End + + It "fails on ERROR 1231 (wrong value)" + mysql() { + echo "ERROR 1231 (42000): Variable 'innodb_flush_log_at_trx_commit' can't be set to the value of '9'" >&2 + return 1 + } + When run source ../scripts/update-parameter.sh innodb_flush_log_at_trx_commit 9 + The status should be failure + The stderr should include "Failed to set parameter" + End + + It "fails on authentication error (ERROR 1045)" + mysql() { + echo "ERROR 1045 (28000): Access denied for user 'kbadmin'@'localhost'" >&2 + return 1 + } + When run source ../scripts/update-parameter.sh max_connections 500 + The status should be failure + The stderr should include "Failed to set parameter" + End + + It "fails when the server is unreachable" + mysql() { + echo "ERROR 2003 (HY000): Can't connect to MySQL server on '127.0.0.1:3306'" >&2 + return 1 + } + When run source ../scripts/update-parameter.sh max_connections 500 + The status should be failure + The stderr should include "Failed to set parameter" + End + End +End From fbe80c6a94eec1b55a6658b7565c32038b864e64 Mon Sep 17 00:00:00 2001 From: wei Date: Tue, 7 Jul 2026 18:32:35 +0800 Subject: [PATCH 3/3] fix(mysql): reject MySQL 8.4-removed variables in update-parameter.sh A reconfigure of a variable removed in MySQL 8.4 (expire_logs_days, master_info_repository, etc.) previously fell through to the config file path and landed in my.cnf, crash-looping 8.4 on restart. Reject such a request by name before SET GLOBAL when the server is >= 8.4, with a clear reason surfaced in the action output. The SET GLOBAL unknown-variable error remains a backstop; 8.0 keeps the legacy variables valid. Adds ShellSpec coverage for 8.4-reject / 8.4-accept-non-removed / 8.0-legacy. Refs #3085 Co-Authored-By: Claude Fable 5 --- .../scripts-ut-spec/update_parameter_spec.sh | 45 +++++++++++++++++++ addons/mysql/scripts/update-parameter.sh | 20 +++++++++ 2 files changed, 65 insertions(+) diff --git a/addons/mysql/scripts-ut-spec/update_parameter_spec.sh b/addons/mysql/scripts-ut-spec/update_parameter_spec.sh index 99d4a849d6..5e02f05ef8 100644 --- a/addons/mysql/scripts-ut-spec/update_parameter_spec.sh +++ b/addons/mysql/scripts-ut-spec/update_parameter_spec.sh @@ -114,4 +114,49 @@ Describe "MySQL update-parameter Script Tests" The stderr should include "Failed to set parameter" End End + + Describe "MySQL 8.4 removed-variable rejection" + # mysql mock that answers SELECT VERSION() with the configured version and + # records / succeeds any SET GLOBAL. The tail argument is the SQL query. + mock_mysql_version() { + local version="$1"; shift + local q; eval "q=\"\${$#}\"" + case "$q" in + *VERSION*) printf '%s\n' "$version" ;; + *) printf '%s' "$q" > "${MOCK_QUERY_FILE}"; return 0 ;; + esac + } + + It "rejects a removed variable on MySQL 8.4 before SET GLOBAL" + mysql() { mock_mysql_version "8.4.8" "$@"; } + When run source ../scripts/update-parameter.sh expire_logs_days 1 + The status should be failure + The stderr should include "removed in MySQL 8.4" + # SET GLOBAL must not have been attempted for the removed variable. + The contents of file "${MOCK_QUERY_FILE}" should not include "SET GLOBAL expire_logs_days" + End + + It "rejects another removed variable (master_info_repository) on 8.4" + mysql() { mock_mysql_version "8.4.8" "$@"; } + When run source ../scripts/update-parameter.sh master_info_repository TABLE + The status should be failure + The stderr should include "removed in MySQL 8.4" + End + + It "accepts a non-removed variable on MySQL 8.4" + mysql() { mock_mysql_version "8.4.8" "$@"; } + When run source ../scripts/update-parameter.sh max_connections 500 + The status should be success + The stdout should include "Set parameter max_connections" + The contents of file "${MOCK_QUERY_FILE}" should equal "SET GLOBAL max_connections = 500;" + End + + It "does not reject the removed name on MySQL 8.0 (still a legal 8.0 variable)" + mysql() { mock_mysql_version "8.0.33" "$@"; } + When run source ../scripts/update-parameter.sh expire_logs_days 1 + The status should be success + The stdout should include "Set parameter expire_logs_days" + The contents of file "${MOCK_QUERY_FILE}" should equal "SET GLOBAL expire_logs_days = 1;" + End + End End diff --git a/addons/mysql/scripts/update-parameter.sh b/addons/mysql/scripts/update-parameter.sh index 0fd5db40bc..e7d83064ac 100644 --- a/addons/mysql/scripts/update-parameter.sh +++ b/addons/mysql/scripts/update-parameter.sh @@ -17,6 +17,26 @@ if echo "${paramName}" | grep -q "^loose_"; then fi paramName=$(echo "${paramName}" | tr '-' '_') +# MySQL 8.4 removed several system variables. Reject a reconfigure of any of +# them BEFORE the SET GLOBAL / config write, so it fails explicitly with a +# clear reason instead of silently landing in my.cnf and crash-looping the +# instance on the next restart. The SET GLOBAL unknown-variable error below is +# a backstop; this by-name check makes the rejection deterministic and visible. +mysql_84_removed_vars="expire_logs_days default_authentication_plugin binlog_transaction_dependency_tracking transaction_write_set_extraction slave_rows_search_algorithms master_info_repository relay_log_info_repository log_bin_use_v1_row_events" +server_version=$(mysql_exec "SELECT VERSION();" 2>/dev/null | head -n1) +server_major=$(echo "${server_version}" | cut -d. -f1) +server_minor=$(echo "${server_version}" | cut -d. -f2) +if [[ "${server_major}" =~ ^[0-9]+$ ]] && [[ "${server_minor}" =~ ^[0-9]+$ ]]; then + if [ "${server_major}" -gt 8 ] || { [ "${server_major}" -eq 8 ] && [ "${server_minor}" -ge 4 ]; }; then + for removed in ${mysql_84_removed_vars}; do + if [ "${paramName}" = "${removed}" ]; then + echo "Parameter ${paramName} was removed in MySQL 8.4 and cannot be set on server version ${server_version}; rejecting reconfigure." >&2 + exit 1 + fi + done + fi +fi + var_int=-1 if [[ "${paramValue}" =~ ^[0-9]+$ ]]; then var_int="${paramValue}"