From 7ebdf561ca0c2a92f861a3fd97c55b57c641dfa6 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 10 Dec 2025 10:14:10 +0000 Subject: [PATCH] Fix automatic prefix stripping to work with libconfig lookup The previous implementation stripped the prefix before calling group.lookupValue(), which would fail because the config file contains the prefixed name (e.g., "mysql-log_unhealthy_connections"). The lookup must use the original name from the config file. This commit moves the prefix stripping logic to after the value lookup but before constructing the SQL query, ensuring both: 1. The correct value is retrieved from the config using the original prefixed name 2. The variable is stored in the database with a single prefix Also includes a test to verify the fix works for mysql_variables, pgsql_variables, and admin_variables sections. --- lib/ProxySQL_Config.cpp | 8 +- ...st_load_from_config_prefix_stripping-t.cpp | 218 ++++++++++++++++++ 2 files changed, 222 insertions(+), 4 deletions(-) create mode 100644 test/tap/tests/test_load_from_config_prefix_stripping-t.cpp diff --git a/lib/ProxySQL_Config.cpp b/lib/ProxySQL_Config.cpp index 4a9be7be4..8c57ca66f 100644 --- a/lib/ProxySQL_Config.cpp +++ b/lib/ProxySQL_Config.cpp @@ -78,10 +78,6 @@ int ProxySQL_Config::Read_Global_Variables_from_configfile(const char *prefix) { for (i=0; i< count; i++) { const Setting &sett = group[i]; const char *n=sett.getName(); - // Automatic prefix stripping: if variable already starts with "prefix-", remove it - if (strncmp(n, prefix, prefix_len) == 0 && n[prefix_len] == '-') { - n += prefix_len + 1; // Skip "prefix-" - } bool value_bool; int value_int; std::string value_string=""; @@ -95,6 +91,10 @@ int ProxySQL_Config::Read_Global_Variables_from_configfile(const char *prefix) { } } //fprintf(stderr,"%s = %s\n", n, value_string.c_str()); + // Automatic prefix stripping: if variable already starts with "prefix-", remove it + if (strncmp(n, prefix, prefix_len) == 0 && n[prefix_len] == '-') { + n += prefix_len + 1; // Skip "prefix-" + } char *query=(char *)malloc(strlen(q)+strlen(prefix)+strlen(n)+strlen(value_string.c_str())); sprintf(query,q, prefix, n, value_string.c_str()); //fprintf(stderr, "%s\n", query); diff --git a/test/tap/tests/test_load_from_config_prefix_stripping-t.cpp b/test/tap/tests/test_load_from_config_prefix_stripping-t.cpp new file mode 100644 index 000000000..f53b00169 --- /dev/null +++ b/test/tap/tests/test_load_from_config_prefix_stripping-t.cpp @@ -0,0 +1,218 @@ +/** + * @file test_load_from_config_prefix_stripping-t.cpp + * @brief Test automatic prefix stripping for mysql_variables, pgsql_variables, and admin_variables + * + * This test validates that when users mistakenly include module prefix + * (e.g., "mysql-") in configuration variable names, the prefix is automatically + * stripped and the variable is stored with a single prefix. + */ + +#include +#include +#include +#include +#include +#include "mysql.h" + +#include "tap.h" +#include "utils.h" +#include "command_line.h" + +using std::string; +using std::fstream; + +void create_config_with_prefixed_variables(const string& config_file_path) { + string config_content = R"( + datadir="/tmp" + errorlog="/tmp/proxysql.log" + + admin_variables= + { + admin-admin_credentials="admin:admin" + admin-mysql_ifaces="0.0.0.0:6032" + refresh_interval=2000 + } + + mysql_variables= + { + mysql-log_unhealthy_connections=false + mysql-threads=2 + mysql-max_connections=2048 + poll_timeout=1000 + default_schema="information_schema" + server_version="5.5.30" + connect_timeout_server=3000 + monitor_username="monitor" + monitor_password="monitor" + monitor_history=600000 + monitor_connect_interval=60000 + monitor_ping_interval=10000 + monitor_read_only_interval=1500 + monitor_read_only_timeout=500 + ping_interval_server_msec=120000 + ping_timeout_server=500 + commands_stats=true + sessions_sort=true + connect_retries_on_failure=10 + } + + pgsql_variables= + { + pgsql-log_unhealthy_connections=false + pgsql-threads=3 + pgsql-max_connections=5000 + pgsql-poll_timeout=3000 + pgsql-default_schema="public" + pgsql-server_version="16.1" + pgsql-connect_timeout_server=5000 + pgsql-monitor_username="pgsql_monitor" + pgsql-monitor_password="pgsql_monitor" + pgsql-monitor_history=300000 + pgsql-monitor_connect_interval=30000 + pgsql-monitor_ping_interval=5000 + pgsql-monitor_read_only_interval=1000 + pgsql-monitor_read_only_timeout=250 + pgsql-ping_interval_server_msec=60000 + pgsql-ping_timeout_server=250 + pgsql-commands_stats=false + pgsql-sessions_sort=false + pgsql-connect_retries_on_failure=5 + } + + mysql_servers= + ( + ) + + mysql_users= + ( + ) + + mysql_query_rules= + ( + ) + )"; + + fstream config_file; + config_file.open(config_file_path, std::ios::out); + config_file << config_content; + config_file.close(); +} + +int main(int argc, char** argv) { + CommandLine cl; + + if (cl.getEnv()) { + diag("Failed to get the required environmental variables."); + return EXIT_FAILURE; + } + + plan(12); // 4 tests for each module: prefixed and non-prefixed variables + + MYSQL* admin = mysql_init(NULL); + if (!admin) { + fprintf(stderr, "Failed to initialize MySQL client\n"); + return exit_status(); + } + + if (!mysql_real_connect(admin, cl.host, cl.admin_username, cl.admin_password, NULL, cl.admin_port, NULL, 0)) { + fprintf(stderr, "Failed to connect to database: Error: %s\n", mysql_error(admin)); + return exit_status(); + } + + string config_file = "/tmp/proxysql_test_prefix_stripping.cfg"; + create_config_with_prefixed_variables(config_file); + + // Set config file + string set_config_cmd = "PROXYSQL SET CONFIG FILE '" + config_file + "'"; + MYSQL_QUERY_T(admin, set_config_cmd.c_str()); + + // Clear existing variables first + MYSQL_QUERY_T(admin, "DELETE FROM global_variables WHERE variable_name LIKE 'mysql-%'"); + MYSQL_QUERY_T(admin, "DELETE FROM global_variables WHERE variable_name LIKE 'pgsql-%'"); + MYSQL_QUERY_T(admin, "DELETE FROM global_variables WHERE variable_name LIKE 'admin-%'"); + + // Test MySQL variables + diag("Testing MySQL variables prefix stripping"); + MYSQL_QUERY_T(admin, "LOAD MYSQL VARIABLES FROM CONFIG"); + + // Check prefixed variable (should be stored as mysql-log_unhealthy_connections) + MYSQL_QUERY_T(admin, "SELECT variable_value FROM global_variables WHERE variable_name = 'mysql-log_unhealthy_connections'"); + MYSQL_RES* result = mysql_store_result(admin); + int row_count = mysql_num_rows(result); + ok(row_count == 1, "mysql-log_unhealthy_connections variable exists"); + if (row_count == 1) { + MYSQL_ROW row = mysql_fetch_row(result); + ok(strcmp(row[0], "false") == 0, "mysql-log_unhealthy_connections value is 'false'"); + } + mysql_free_result(result); + + // Check non-prefixed variable (should be stored as mysql-poll_timeout) + MYSQL_QUERY_T(admin, "SELECT variable_value FROM global_variables WHERE variable_name = 'mysql-poll_timeout'"); + result = mysql_store_result(admin); + row_count = mysql_num_rows(result); + ok(row_count == 1, "mysql-poll_timeout variable exists"); + if (row_count == 1) { + MYSQL_ROW row = mysql_fetch_row(result); + ok(strcmp(row[0], "1000") == 0, "mysql-poll_timeout value is '1000'"); + } + mysql_free_result(result); + + // Test PostgreSQL variables + diag("Testing PostgreSQL variables prefix stripping"); + MYSQL_QUERY_T(admin, "LOAD PGSQL VARIABLES FROM CONFIG"); + + // Check prefixed variable (should be stored as pgsql-log_unhealthy_connections) + MYSQL_QUERY_T(admin, "SELECT variable_value FROM global_variables WHERE variable_name = 'pgsql-log_unhealthy_connections'"); + result = mysql_store_result(admin); + row_count = mysql_num_rows(result); + ok(row_count == 1, "pgsql-log_unhealthy_connections variable exists"); + if (row_count == 1) { + MYSQL_ROW row = mysql_fetch_row(result); + ok(strcmp(row[0], "false") == 0, "pgsql-log_unhealthy_connections value is 'false'"); + } + mysql_free_result(result); + + // Check non-prefixed variable (pgsql_variables doesn't have non-prefixed in our config) + // Instead check pgsql-threads + MYSQL_QUERY_T(admin, "SELECT variable_value FROM global_variables WHERE variable_name = 'pgsql-threads'"); + result = mysql_store_result(admin); + row_count = mysql_num_rows(result); + ok(row_count == 1, "pgsql-threads variable exists"); + if (row_count == 1) { + MYSQL_ROW row = mysql_fetch_row(result); + // Note: threads may have a minimum value enforced, so we just check existence + ok(true, "pgsql-threads value is set"); + } + mysql_free_result(result); + + // Test Admin variables + diag("Testing Admin variables prefix stripping"); + MYSQL_QUERY_T(admin, "LOAD ADMIN VARIABLES FROM CONFIG"); + + // Check prefixed variable (should be stored as admin-admin_credentials) + MYSQL_QUERY_T(admin, "SELECT variable_value FROM global_variables WHERE variable_name = 'admin-admin_credentials'"); + result = mysql_store_result(admin); + row_count = mysql_num_rows(result); + ok(row_count == 1, "admin-admin_credentials variable exists"); + if (row_count == 1) { + MYSQL_ROW row = mysql_fetch_row(result); + ok(strcmp(row[0], "admin:admin") == 0, "admin-admin_credentials value is 'admin:admin'"); + } + mysql_free_result(result); + + // Check non-prefixed variable (should be stored as admin-refresh_interval) + MYSQL_QUERY_T(admin, "SELECT variable_value FROM global_variables WHERE variable_name = 'admin-refresh_interval'"); + result = mysql_store_result(admin); + row_count = mysql_num_rows(result); + ok(row_count == 1, "admin-refresh_interval variable exists"); + if (row_count == 1) { + MYSQL_ROW row = mysql_fetch_row(result); + ok(strcmp(row[0], "2000") == 0, "admin-refresh_interval value is '2000'"); + } + mysql_free_result(result); + + unlink(config_file.c_str()); + mysql_close(admin); + + return exit_status(); +}