From fb11a729cb02e01cbfc622babf5560268324bebd Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 3 Apr 2026 13:14:21 +0000 Subject: [PATCH 01/12] feat: add PgBouncer config parser library (Stage 1) (#5563) Add a standalone C++ library that parses PgBouncer configuration files into structured data, as the foundation for PgBouncer-to-ProxySQL migration tooling. Components: - PgBouncer_Config.h: Data structures for all PgBouncer config entities - PgBouncer_ConfigParser: INI parser for pgbouncer.ini with all 4 sections ([pgbouncer], [databases], [users], [peers]), %include directive support, and connection string parsing - PgBouncer_AuthFileParser: Parser for userlist.txt with MD5/SCRAM/plain password type detection and double-quote escaping - PgBouncer_HBAParser: Parser for pg_hba.conf with support for all connection types, auth methods, and key=value options The parser library has zero ProxySQL dependencies - it uses only the C++ standard library. This makes it independently testable and reusable. Unit test covers: minimal config, full config with all sections, auth file formats, HBA rules, malformed configs (strict error detection), %include directives, nonexistent files, and default value verification (127 tests). --- include/PgBouncer_Config.h | 224 ++++++ lib/Makefile | 15 +- .../PgBouncer_AuthFileParser.cpp | 158 ++++ .../PgBouncer_AuthFileParser.h | 35 + .../PgBouncer_ConfigParser.cpp | 697 ++++++++++++++++++ lib/pgbouncer_compat/PgBouncer_ConfigParser.h | 57 ++ lib/pgbouncer_compat/PgBouncer_HBAParser.cpp | 215 ++++++ lib/pgbouncer_compat/PgBouncer_HBAParser.h | 37 + test/tap/tests/unit/Makefile | 14 +- .../unit/fixtures/pgbouncer_compat/full.ini | 52 ++ .../pgbouncer_compat/include_databases.ini | 2 + .../pgbouncer_compat/include_main.ini | 5 + .../fixtures/pgbouncer_compat/malformed.ini | 9 + .../fixtures/pgbouncer_compat/minimal.ini | 6 + .../fixtures/pgbouncer_compat/pg_hba.conf | 6 + .../fixtures/pgbouncer_compat/userlist.txt | 4 + .../unit/pgbouncer_config_parser_unit-t.cpp | 364 +++++++++ 17 files changed, 1898 insertions(+), 2 deletions(-) create mode 100644 include/PgBouncer_Config.h create mode 100644 lib/pgbouncer_compat/PgBouncer_AuthFileParser.cpp create mode 100644 lib/pgbouncer_compat/PgBouncer_AuthFileParser.h create mode 100644 lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp create mode 100644 lib/pgbouncer_compat/PgBouncer_ConfigParser.h create mode 100644 lib/pgbouncer_compat/PgBouncer_HBAParser.cpp create mode 100644 lib/pgbouncer_compat/PgBouncer_HBAParser.h create mode 100644 test/tap/tests/unit/fixtures/pgbouncer_compat/full.ini create mode 100644 test/tap/tests/unit/fixtures/pgbouncer_compat/include_databases.ini create mode 100644 test/tap/tests/unit/fixtures/pgbouncer_compat/include_main.ini create mode 100644 test/tap/tests/unit/fixtures/pgbouncer_compat/malformed.ini create mode 100644 test/tap/tests/unit/fixtures/pgbouncer_compat/minimal.ini create mode 100644 test/tap/tests/unit/fixtures/pgbouncer_compat/pg_hba.conf create mode 100644 test/tap/tests/unit/fixtures/pgbouncer_compat/userlist.txt create mode 100644 test/tap/tests/unit/pgbouncer_config_parser_unit-t.cpp diff --git a/include/PgBouncer_Config.h b/include/PgBouncer_Config.h new file mode 100644 index 0000000000..34e151f1e1 --- /dev/null +++ b/include/PgBouncer_Config.h @@ -0,0 +1,224 @@ +#ifndef PGBOUNCER_CONFIG_H +#define PGBOUNCER_CONFIG_H + +#include +#include +#include + +namespace PgBouncer { + +struct ParseMessage { + std::string file; + int line = 0; + std::string message; +}; + +struct GlobalSettings { + // Listener + std::string listen_addr; + int listen_port = 6432; + std::string unix_socket_dir; + int listen_backlog = 128; + std::string unix_socket_mode; + std::string unix_socket_group; + + // Pooling + std::string pool_mode = "session"; + int default_pool_size = 20; + int min_pool_size = 0; + int reserve_pool_size = 0; + int reserve_pool_timeout = 5; + bool server_round_robin = false; + + // Limits + int max_client_conn = 100; + int max_db_connections = 0; + int max_db_client_connections = 0; + int max_user_connections = 0; + int max_user_client_connections = 0; + + // Authentication + std::string auth_type = "md5"; + std::string auth_file; + std::string auth_hba_file; + std::string auth_ident_file; + std::string auth_user; + std::string auth_query; + std::string auth_dbname; + std::string auth_ldap_options; + + // Timeouts (in seconds) + int server_lifetime = 3600; + int server_idle_timeout = 600; + int server_connect_timeout = 15; + int server_login_retry = 15; + int client_login_timeout = 60; + int client_idle_timeout = 0; + int query_timeout = 0; + int query_wait_timeout = 120; + int cancel_wait_timeout = 10; + int idle_transaction_timeout = 0; + int transaction_timeout = 0; + int suspend_timeout = 10; + int autodb_idle_timeout = 3600; + + // Server maintenance + std::string server_reset_query = "DISCARD ALL"; + bool server_reset_query_always = false; + std::string server_check_query; + int server_check_delay = 30; + bool server_fast_close = false; + + // TLS client-facing + std::string client_tls_sslmode = "disable"; + std::string client_tls_key_file; + std::string client_tls_cert_file; + std::string client_tls_ca_file; + std::string client_tls_protocols = "secure"; + std::string client_tls_ciphers; + std::string client_tls13_ciphers; + std::string client_tls_dheparams = "auto"; + std::string client_tls_ecdhcurve = "auto"; + + // TLS server-facing + std::string server_tls_sslmode = "prefer"; + std::string server_tls_key_file; + std::string server_tls_cert_file; + std::string server_tls_ca_file; + std::string server_tls_protocols = "secure"; + std::string server_tls_ciphers; + std::string server_tls13_ciphers; + + // Logging + std::string logfile; + bool syslog = false; + std::string syslog_ident = "pgbouncer"; + std::string syslog_facility = "daemon"; + bool log_connections = true; + bool log_disconnections = true; + bool log_pooler_errors = true; + bool log_stats = true; + int stats_period = 60; + int verbose = 0; + + // Admin + std::string admin_users; + std::string stats_users; + + // Networking + bool so_reuseport = false; + bool tcp_defer_accept = false; + bool tcp_keepalive = true; + int tcp_keepcnt = 0; + int tcp_keepidle = 0; + int tcp_keepintvl = 0; + int tcp_socket_buffer = 0; + int tcp_user_timeout = 0; + + // Protocol + int max_prepared_statements = 200; + bool disable_pqexec = false; + bool application_name_add_host = false; + std::string track_extra_parameters = "IntervalStyle"; + std::string ignore_startup_parameters; + int scram_iterations = 4096; + int pkt_buf = 4096; + unsigned int max_packet_size = 2147483647; + int sbuf_loopcnt = 5; + int query_wait_notify = 5; + + // DNS + int dns_max_ttl = 15; + int dns_nxdomain_ttl = 15; + int dns_zone_check_period = 0; + std::string resolv_conf; + + // Process + std::string pidfile; + std::string user; + int peer_id = 0; +}; + +struct Database { + std::string name; // entry name, or "*" for wildcard + std::string host; // comma-separated for multi-host + int port = 5432; + std::string dbname; // destination database (empty = same as name) + std::string user; // forced user (empty = client user) + std::string password; + std::string auth_user; + std::string auth_query; + std::string auth_dbname; + std::string pool_mode; // per-db override, empty = use global + int pool_size = -1; // -1 = use default + int min_pool_size = -1; + int reserve_pool_size = -1; + int max_db_connections = -1; + int max_db_client_connections = -1; + int server_lifetime = -1; + std::string load_balance_hosts; + std::string connect_query; + std::string client_encoding; + std::string datestyle; + std::string timezone; + std::string application_name; +}; + +struct User { + std::string name; + std::string pool_mode; + int pool_size = -1; + int reserve_pool_size = -1; + int max_user_connections = -1; + int max_user_client_connections = -1; + int query_timeout = -1; + int idle_transaction_timeout = -1; + int transaction_timeout = -1; + int client_idle_timeout = -1; +}; + +struct Peer { + int peer_id = 0; + std::string host; + int port = 6432; + int pool_size = -1; +}; + +enum class AuthType { PLAIN, MD5, SCRAM }; + +struct AuthFileEntry { + std::string username; + std::string password; + AuthType type = AuthType::PLAIN; +}; + +struct HBARule { + std::string conn_type; // local, host, hostssl, hostnossl + std::string database; // all, sameuser, dbname, @file + std::string user; // all, username, @file + std::string address; // IP/CIDR or "all" (empty for local) + std::string mask; // optional separate mask + std::string method; // trust, reject, md5, scram-sha-256, etc. + std::map options; +}; + +struct Config { + GlobalSettings global; + std::vector databases; + std::vector users; + std::vector peers; + std::vector auth_entries; + std::vector hba_rules; + std::vector errors; + std::vector warnings; +}; + +// Parser functions +// Returns true on success, false on error (errors populated in config.errors) +bool parse_config_file(const std::string& filepath, Config& config); +bool parse_auth_file(const std::string& filepath, std::vector& entries, std::vector& errors); +bool parse_hba_file(const std::string& filepath, std::vector& rules, std::vector& errors); + +} // namespace PgBouncer + +#endif // PGBOUNCER_CONFIG_H diff --git a/lib/Makefile b/lib/Makefile index 63bcc297c4..8d8e93dd10 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -124,7 +124,10 @@ _OBJ_CXX := ProxySQL_GloVars.oo network.oo debug.oo configfile.oo Query_Cache.oo MySQLErrorClassifier.oo \ BackendSyncDecision.oo \ Query_Processor_ParserSQL.oo \ - proxy_sqlite3_symbols.oo + proxy_sqlite3_symbols.oo \ + PgBouncer_ConfigParser.oo \ + PgBouncer_AuthFileParser.oo \ + PgBouncer_HBAParser.oo ifeq ($(PROXYSQL31),1) _OBJ_CXX += MySQL_Caching_Sha2_RSA.oo @@ -154,6 +157,16 @@ HEADERS := ../include/*.h ../include/*.hpp $(ODIR)/proxy_sqlite3_symbols.oo: proxy_sqlite3_symbols.cpp $(HEADERS) $(CXX) -fPIC -c -o $@ $< $(MYCXXFLAGS) $(CXXFLAGS) -DSQLITE_CORE -DSQLITE_VEC_STATIC +# PgBouncer compatibility module +$(ODIR)/PgBouncer_ConfigParser.oo: pgbouncer_compat/PgBouncer_ConfigParser.cpp $(HEADERS) + $(CXX) -fPIC -c -o $@ $< $(MYCXXFLAGS) $(CXXFLAGS) -Ipgbouncer_compat + +$(ODIR)/PgBouncer_AuthFileParser.oo: pgbouncer_compat/PgBouncer_AuthFileParser.cpp $(HEADERS) + $(CXX) -fPIC -c -o $@ $< $(MYCXXFLAGS) $(CXXFLAGS) -Ipgbouncer_compat + +$(ODIR)/PgBouncer_HBAParser.oo: pgbouncer_compat/PgBouncer_HBAParser.cpp $(HEADERS) + $(CXX) -fPIC -c -o $@ $< $(MYCXXFLAGS) $(CXXFLAGS) -Ipgbouncer_compat + $(ODIR)/%.oo: %.cpp $(HEADERS) $(CXX) -fPIC -c -o $@ $< $(MYCXXFLAGS) $(CXXFLAGS) diff --git a/lib/pgbouncer_compat/PgBouncer_AuthFileParser.cpp b/lib/pgbouncer_compat/PgBouncer_AuthFileParser.cpp new file mode 100644 index 0000000000..74a27ff44d --- /dev/null +++ b/lib/pgbouncer_compat/PgBouncer_AuthFileParser.cpp @@ -0,0 +1,158 @@ +#include "PgBouncer_AuthFileParser.h" + +#include +#include +#include + +namespace PgBouncer { + +// --------------------------------------------------------------------------- +// AuthFileParser – private helpers +// --------------------------------------------------------------------------- + +bool AuthFileParser::parse_quoted_string(const std::string& line, size_t& pos, + std::string& result) { + result.clear(); + + // Must start with a double-quote + if (pos >= line.size() || line[pos] != '"') { + return false; + } + ++pos; // skip opening quote + + while (pos < line.size()) { + char ch = line[pos]; + if (ch == '"') { + // Check for escaped quote ("") + if (pos + 1 < line.size() && line[pos + 1] == '"') { + result += '"'; + pos += 2; + } else { + // Closing quote + ++pos; + return true; + } + } else { + result += ch; + ++pos; + } + } + + // Reached end of line without closing quote + return false; +} + +AuthType AuthFileParser::detect_password_type(const std::string& password) { + // SCRAM detection: starts with "SCRAM-SHA-256$" + if (password.size() >= 14 && password.compare(0, 14, "SCRAM-SHA-256$") == 0) { + return AuthType::SCRAM; + } + + // MD5 detection: exactly "md5" + 32 hex characters = 35 chars total + if (password.size() == 35 && + password[0] == 'm' && password[1] == 'd' && password[2] == '5') { + bool all_hex = true; + for (size_t i = 3; i < 35; ++i) { + char c = password[i]; + if (!((c >= '0' && c <= '9') || + (c >= 'a' && c <= 'f') || + (c >= 'A' && c <= 'F'))) { + all_hex = false; + break; + } + } + if (all_hex) { + return AuthType::MD5; + } + } + + return AuthType::PLAIN; +} + +// --------------------------------------------------------------------------- +// AuthFileParser::parse +// --------------------------------------------------------------------------- + +bool AuthFileParser::parse(const std::string& filepath, + std::vector& entries, + std::vector& errors) { + std::ifstream file(filepath); + if (!file.is_open()) { + errors.push_back({filepath, 0, "Cannot open auth file: " + filepath}); + return false; + } + + std::string line; + int line_num = 0; + bool had_errors = false; + + while (std::getline(file, line)) { + ++line_num; + + // Skip empty lines + if (line.empty()) { + continue; + } + + // Find the first non-whitespace character + size_t first_non_ws = line.find_first_not_of(" \t\r"); + if (first_non_ws == std::string::npos) { + // Blank line (only whitespace) + continue; + } + + // Skip comment lines (starting with ; or #) + char first_char = line[first_non_ws]; + if (first_char == ';' || first_char == '#') { + continue; + } + + // Parse username + size_t pos = first_non_ws; + std::string username; + if (!parse_quoted_string(line, pos, username)) { + errors.push_back({filepath, line_num, + "Malformed username (expected double-quoted string)"}); + had_errors = true; + continue; + } + + // Skip whitespace between username and password + while (pos < line.size() && (line[pos] == ' ' || line[pos] == '\t')) { + ++pos; + } + + // Parse password + std::string password; + if (!parse_quoted_string(line, pos, password)) { + errors.push_back({filepath, line_num, + "Malformed password (expected double-quoted string)"}); + had_errors = true; + continue; + } + + // Extra fields after the second quoted string are silently ignored, + // matching PgBouncer behaviour. + + AuthFileEntry entry; + entry.username = std::move(username); + entry.password = std::move(password); + entry.type = detect_password_type(entry.password); + entries.push_back(std::move(entry)); + } + + return !had_errors; +} + +// --------------------------------------------------------------------------- +// Free function declared in PgBouncer_Config.h +// --------------------------------------------------------------------------- + +bool parse_auth_file(const std::string& filepath, + std::vector& entries, + std::vector& errors) { + AuthFileParser parser; + return parser.parse(filepath, entries, errors); +} + +} // namespace PgBouncer diff --git a/lib/pgbouncer_compat/PgBouncer_AuthFileParser.h b/lib/pgbouncer_compat/PgBouncer_AuthFileParser.h new file mode 100644 index 0000000000..29f27f15bb --- /dev/null +++ b/lib/pgbouncer_compat/PgBouncer_AuthFileParser.h @@ -0,0 +1,35 @@ +#ifndef PGBOUNCER_AUTH_FILE_PARSER_H +#define PGBOUNCER_AUTH_FILE_PARSER_H + +#include "PgBouncer_Config.h" +#include +#include + +namespace PgBouncer { + +class AuthFileParser { +public: + // Parse a PgBouncer userlist.txt file. + // Format: "username" "password" per line + // Password types detected: + // - Plain text: any string not matching MD5 or SCRAM patterns + // - MD5: starts with "md5" followed by 32 hex chars + // - SCRAM: starts with "SCRAM-SHA-256$" + // Double-quote escaping: "" inside quoted strings represents a literal " + bool parse(const std::string& filepath, + std::vector& entries, + std::vector& errors); + +private: + // Parse a double-quoted string starting at pos, advancing pos past the closing quote. + // Returns the unescaped content. Returns false if malformed. + static bool parse_quoted_string(const std::string& line, size_t& pos, + std::string& result); + + // Detect password type from the raw password string + static AuthType detect_password_type(const std::string& password); +}; + +} // namespace PgBouncer + +#endif diff --git a/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp b/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp new file mode 100644 index 0000000000..54852069ba --- /dev/null +++ b/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp @@ -0,0 +1,697 @@ +#include "PgBouncer_ConfigParser.h" + +#include +#include +#include +#include + +namespace PgBouncer { + +// --------------------------------------------------------------------------- +// String utilities +// --------------------------------------------------------------------------- + +std::string ConfigParser::trim(const std::string& s) { + auto start = s.find_first_not_of(" \t\r\n"); + if (start == std::string::npos) return ""; + auto end = s.find_last_not_of(" \t\r\n"); + return s.substr(start, end - start + 1); +} + +std::string ConfigParser::unquote(const std::string& s) { + if (s.size() >= 2 && s.front() == '\'' && s.back() == '\'') { + // PgBouncer single-quote escaping: '' -> ' + std::string result; + result.reserve(s.size()); + for (size_t i = 1; i + 1 < s.size(); ++i) { + if (s[i] == '\'' && i + 2 < s.size() && s[i + 1] == '\'') { + result += '\''; + ++i; + } else { + result += s[i]; + } + } + return result; + } + return s; +} + +bool ConfigParser::parse_bool(const std::string& value, bool& result) { + std::string lower = value; + std::transform(lower.begin(), lower.end(), lower.begin(), + [](unsigned char c) { return std::tolower(c); }); + if (lower == "1" || lower == "yes" || lower == "true" || lower == "on") { + result = true; + return true; + } + if (lower == "0" || lower == "no" || lower == "false" || lower == "off") { + result = false; + return true; + } + return false; +} + +bool ConfigParser::parse_int(const std::string& value, int& result) { + if (value.empty()) return false; + try { + size_t pos = 0; + long v = std::stol(value, &pos); + if (pos != value.size()) return false; + result = static_cast(v); + return true; + } catch (...) { + return false; + } +} + +bool ConfigParser::parse_uint(const std::string& value, unsigned int& result) { + if (value.empty()) return false; + try { + size_t pos = 0; + unsigned long v = std::stoul(value, &pos); + if (pos != value.size()) return false; + result = static_cast(v); + return true; + } catch (...) { + return false; + } +} + +// --------------------------------------------------------------------------- +// Connection-string pair parser +// --------------------------------------------------------------------------- + +bool ConfigParser::parse_connstr_pairs( + const std::string& connstr, + std::vector>& pairs, + const std::string& file, int line, + std::vector& errors) +{ + // Parses "key=value key2=value2 key3='val with spaces'" + pairs.clear(); + size_t i = 0; + const size_t len = connstr.size(); + + while (i < len) { + // Skip whitespace + while (i < len && std::isspace(static_cast(connstr[i]))) ++i; + if (i >= len) break; + + // Read key + size_t key_start = i; + while (i < len && connstr[i] != '=' && !std::isspace(static_cast(connstr[i]))) ++i; + std::string key = connstr.substr(key_start, i - key_start); + + // Skip whitespace around '=' + while (i < len && std::isspace(static_cast(connstr[i]))) ++i; + if (i >= len || connstr[i] != '=') { + errors.push_back({file, line, "expected '=' after key '" + key + "' in connection string"}); + return false; + } + ++i; // skip '=' + while (i < len && std::isspace(static_cast(connstr[i]))) ++i; + + // Read value + std::string value; + if (i < len && connstr[i] == '\'') { + // Quoted value: collect until unescaped closing quote + // PgBouncer escapes single quotes by doubling: '' + ++i; // skip opening quote + std::string raw = "'"; + while (i < len) { + if (connstr[i] == '\'') { + if (i + 1 < len && connstr[i + 1] == '\'') { + raw += "''"; + i += 2; + } else { + // closing quote + ++i; + break; + } + } else { + raw += connstr[i]; + ++i; + } + } + raw += "'"; + value = unquote(raw); + } else { + // Unquoted value: read until whitespace + size_t val_start = i; + while (i < len && !std::isspace(static_cast(connstr[i]))) ++i; + value = connstr.substr(val_start, i - val_start); + } + + pairs.emplace_back(std::move(key), std::move(value)); + } + return true; +} + +// --------------------------------------------------------------------------- +// Section parsers +// --------------------------------------------------------------------------- + +bool ConfigParser::parse_global_key( + const std::string& key, const std::string& value, + GlobalSettings& settings, const std::string& file, int line, + std::vector& errors) +{ + // Helper macros to reduce repetition. + // Each maps a key name to the corresponding field. +#define SET_STR(name) do { if (key == #name) { settings.name = value; return true; } } while(0) +#define SET_INT(name) do { if (key == #name) { \ + int v; if (!parse_int(value, v)) { \ + errors.push_back({file, line, "invalid integer for '" + key + "': " + value}); return false; } \ + settings.name = v; return true; } } while(0) +#define SET_UINT(name) do { if (key == #name) { \ + unsigned int v; if (!parse_uint(value, v)) { \ + errors.push_back({file, line, "invalid unsigned integer for '" + key + "': " + value}); return false; } \ + settings.name = v; return true; } } while(0) +#define SET_BOOL(name) do { if (key == #name) { \ + bool v; if (!parse_bool(value, v)) { \ + errors.push_back({file, line, "invalid boolean for '" + key + "': " + value}); return false; } \ + settings.name = v; return true; } } while(0) + + // Listener + SET_STR(listen_addr); + SET_INT(listen_port); + SET_STR(unix_socket_dir); + SET_INT(listen_backlog); + SET_STR(unix_socket_mode); + SET_STR(unix_socket_group); + + // Pooling + SET_STR(pool_mode); + SET_INT(default_pool_size); + SET_INT(min_pool_size); + SET_INT(reserve_pool_size); + SET_INT(reserve_pool_timeout); + SET_BOOL(server_round_robin); + + // Limits + SET_INT(max_client_conn); + SET_INT(max_db_connections); + SET_INT(max_db_client_connections); + SET_INT(max_user_connections); + SET_INT(max_user_client_connections); + + // Authentication + SET_STR(auth_type); + SET_STR(auth_file); + SET_STR(auth_hba_file); + SET_STR(auth_ident_file); + SET_STR(auth_user); + SET_STR(auth_query); + SET_STR(auth_dbname); + SET_STR(auth_ldap_options); + + // Timeouts + SET_INT(server_lifetime); + SET_INT(server_idle_timeout); + SET_INT(server_connect_timeout); + SET_INT(server_login_retry); + SET_INT(client_login_timeout); + SET_INT(client_idle_timeout); + SET_INT(query_timeout); + SET_INT(query_wait_timeout); + SET_INT(cancel_wait_timeout); + SET_INT(idle_transaction_timeout); + SET_INT(transaction_timeout); + SET_INT(suspend_timeout); + SET_INT(autodb_idle_timeout); + + // Server maintenance + SET_STR(server_reset_query); + SET_BOOL(server_reset_query_always); + SET_STR(server_check_query); + SET_INT(server_check_delay); + SET_BOOL(server_fast_close); + + // TLS client-facing + SET_STR(client_tls_sslmode); + SET_STR(client_tls_key_file); + SET_STR(client_tls_cert_file); + SET_STR(client_tls_ca_file); + SET_STR(client_tls_protocols); + SET_STR(client_tls_ciphers); + SET_STR(client_tls13_ciphers); + SET_STR(client_tls_dheparams); + SET_STR(client_tls_ecdhcurve); + + // TLS server-facing + SET_STR(server_tls_sslmode); + SET_STR(server_tls_key_file); + SET_STR(server_tls_cert_file); + SET_STR(server_tls_ca_file); + SET_STR(server_tls_protocols); + SET_STR(server_tls_ciphers); + SET_STR(server_tls13_ciphers); + + // Logging + SET_STR(logfile); + SET_BOOL(syslog); + SET_STR(syslog_ident); + SET_STR(syslog_facility); + SET_BOOL(log_connections); + SET_BOOL(log_disconnections); + SET_BOOL(log_pooler_errors); + SET_BOOL(log_stats); + SET_INT(stats_period); + SET_INT(verbose); + + // Admin + SET_STR(admin_users); + SET_STR(stats_users); + + // Networking + SET_BOOL(so_reuseport); + SET_BOOL(tcp_defer_accept); + SET_BOOL(tcp_keepalive); + SET_INT(tcp_keepcnt); + SET_INT(tcp_keepidle); + SET_INT(tcp_keepintvl); + SET_INT(tcp_socket_buffer); + SET_INT(tcp_user_timeout); + + // Protocol + SET_INT(max_prepared_statements); + SET_BOOL(disable_pqexec); + SET_BOOL(application_name_add_host); + SET_STR(track_extra_parameters); + SET_STR(ignore_startup_parameters); + SET_INT(scram_iterations); + SET_INT(pkt_buf); + SET_UINT(max_packet_size); + SET_INT(sbuf_loopcnt); + SET_INT(query_wait_notify); + + // DNS + SET_INT(dns_max_ttl); + SET_INT(dns_nxdomain_ttl); + SET_INT(dns_zone_check_period); + SET_STR(resolv_conf); + + // Process + SET_STR(pidfile); + SET_STR(user); + SET_INT(peer_id); + +#undef SET_STR +#undef SET_INT +#undef SET_UINT +#undef SET_BOOL + + errors.push_back({file, line, "unknown pgbouncer setting: " + key}); + return false; +} + +bool ConfigParser::parse_database_entry( + const std::string& name, const std::string& connstr, + Database& db, const std::string& file, int line, + std::vector& errors) +{ + db.name = name; + + std::vector> pairs; + if (!parse_connstr_pairs(connstr, pairs, file, line, errors)) { + return false; + } + + bool ok = true; + for (const auto& [k, v] : pairs) { + if (k == "host") { db.host = v; } + else if (k == "port") { + if (!parse_int(v, db.port)) { + errors.push_back({file, line, "invalid integer for 'port': " + v}); + ok = false; + } + } + else if (k == "dbname") { db.dbname = v; } + else if (k == "user") { db.user = v; } + else if (k == "password") { db.password = v; } + else if (k == "auth_user") { db.auth_user = v; } + else if (k == "auth_query") { db.auth_query = v; } + else if (k == "auth_dbname") { db.auth_dbname = v; } + else if (k == "pool_mode") { db.pool_mode = v; } + else if (k == "pool_size") { + if (!parse_int(v, db.pool_size)) { + errors.push_back({file, line, "invalid integer for 'pool_size': " + v}); + ok = false; + } + } + else if (k == "min_pool_size") { + if (!parse_int(v, db.min_pool_size)) { + errors.push_back({file, line, "invalid integer for 'min_pool_size': " + v}); + ok = false; + } + } + else if (k == "reserve_pool_size") { + if (!parse_int(v, db.reserve_pool_size)) { + errors.push_back({file, line, "invalid integer for 'reserve_pool_size': " + v}); + ok = false; + } + } + else if (k == "max_db_connections") { + if (!parse_int(v, db.max_db_connections)) { + errors.push_back({file, line, "invalid integer for 'max_db_connections': " + v}); + ok = false; + } + } + else if (k == "max_db_client_connections") { + if (!parse_int(v, db.max_db_client_connections)) { + errors.push_back({file, line, "invalid integer for 'max_db_client_connections': " + v}); + ok = false; + } + } + else if (k == "server_lifetime") { + if (!parse_int(v, db.server_lifetime)) { + errors.push_back({file, line, "invalid integer for 'server_lifetime': " + v}); + ok = false; + } + } + else if (k == "load_balance_hosts") { db.load_balance_hosts = v; } + else if (k == "connect_query") { db.connect_query = v; } + else if (k == "client_encoding") { db.client_encoding = v; } + else if (k == "datestyle") { db.datestyle = v; } + else if (k == "timezone") { db.timezone = v; } + else if (k == "application_name") { db.application_name = v; } + else { + errors.push_back({file, line, "unknown database parameter: " + k}); + ok = false; + } + } + return ok; +} + +bool ConfigParser::parse_user_entry( + const std::string& name, const std::string& settings_str, + User& user, const std::string& file, int line, + std::vector& errors) +{ + user.name = name; + + std::vector> pairs; + if (!parse_connstr_pairs(settings_str, pairs, file, line, errors)) { + return false; + } + + bool ok = true; + for (const auto& [k, v] : pairs) { + if (k == "pool_mode") { user.pool_mode = v; } + else if (k == "pool_size") { + if (!parse_int(v, user.pool_size)) { + errors.push_back({file, line, "invalid integer for 'pool_size': " + v}); + ok = false; + } + } + else if (k == "reserve_pool_size") { + if (!parse_int(v, user.reserve_pool_size)) { + errors.push_back({file, line, "invalid integer for 'reserve_pool_size': " + v}); + ok = false; + } + } + else if (k == "max_user_connections") { + if (!parse_int(v, user.max_user_connections)) { + errors.push_back({file, line, "invalid integer for 'max_user_connections': " + v}); + ok = false; + } + } + else if (k == "max_user_client_connections") { + if (!parse_int(v, user.max_user_client_connections)) { + errors.push_back({file, line, "invalid integer for 'max_user_client_connections': " + v}); + ok = false; + } + } + else if (k == "query_timeout") { + if (!parse_int(v, user.query_timeout)) { + errors.push_back({file, line, "invalid integer for 'query_timeout': " + v}); + ok = false; + } + } + else if (k == "idle_transaction_timeout") { + if (!parse_int(v, user.idle_transaction_timeout)) { + errors.push_back({file, line, "invalid integer for 'idle_transaction_timeout': " + v}); + ok = false; + } + } + else if (k == "transaction_timeout") { + if (!parse_int(v, user.transaction_timeout)) { + errors.push_back({file, line, "invalid integer for 'transaction_timeout': " + v}); + ok = false; + } + } + else if (k == "client_idle_timeout") { + if (!parse_int(v, user.client_idle_timeout)) { + errors.push_back({file, line, "invalid integer for 'client_idle_timeout': " + v}); + ok = false; + } + } + else { + errors.push_back({file, line, "unknown user parameter: " + k}); + ok = false; + } + } + return ok; +} + +bool ConfigParser::parse_peer_entry( + const std::string& name, const std::string& connstr, + Peer& peer, const std::string& file, int line, + std::vector& errors) +{ + // The name in [peers] is the peer_id (an integer) + if (!parse_int(name, peer.peer_id)) { + errors.push_back({file, line, "invalid peer_id (expected integer): " + name}); + return false; + } + + std::vector> pairs; + if (!parse_connstr_pairs(connstr, pairs, file, line, errors)) { + return false; + } + + bool ok = true; + for (const auto& [k, v] : pairs) { + if (k == "host") { peer.host = v; } + else if (k == "port") { + if (!parse_int(v, peer.port)) { + errors.push_back({file, line, "invalid integer for 'port': " + v}); + ok = false; + } + } + else if (k == "pool_size") { + if (!parse_int(v, peer.pool_size)) { + errors.push_back({file, line, "invalid integer for 'pool_size': " + v}); + ok = false; + } + } + else { + errors.push_back({file, line, "unknown peer parameter: " + k}); + ok = false; + } + } + return ok; +} + +// --------------------------------------------------------------------------- +// Core INI parser +// --------------------------------------------------------------------------- + +bool ConfigParser::parse_ini( + const std::string& filepath, Config& config, + bool resolve_includes, bool resolve_referenced_files) +{ + std::ifstream ifs(filepath); + if (!ifs.is_open()) { + config.errors.push_back({"", 0, "cannot open file: " + filepath}); + return false; + } + + enum class Section { NONE, PGBOUNCER, DATABASES, USERS, PEERS }; + Section current_section = Section::NONE; + + std::string line_str; + int line_num = 0; + bool ok = true; + + // Resolve the directory of this file for relative %include paths + std::string base_dir; + { + auto pos = filepath.find_last_of("/\\"); + if (pos != std::string::npos) { + base_dir = filepath.substr(0, pos + 1); + } + } + + while (std::getline(ifs, line_str)) { + ++line_num; + std::string trimmed = trim(line_str); + + // Skip empty lines and comments + if (trimmed.empty() || trimmed[0] == '#' || trimmed[0] == ';') { + continue; + } + + // %include directive + if (trimmed.size() > 9 && trimmed.substr(0, 9) == "%include ") { + if (!resolve_includes) continue; + std::string inc_path = trim(trimmed.substr(9)); + // Resolve relative paths against the base directory + if (!inc_path.empty() && inc_path[0] != '/') { + inc_path = base_dir + inc_path; + } + if (include_depth_ >= MAX_INCLUDE_DEPTH) { + config.errors.push_back({filepath, line_num, + "maximum include depth (" + std::to_string(MAX_INCLUDE_DEPTH) + ") exceeded"}); + ok = false; + continue; + } + ++include_depth_; + if (!parse_ini(inc_path, config, resolve_includes, resolve_referenced_files)) { + ok = false; + } + --include_depth_; + continue; + } + + // Section header + if (trimmed.front() == '[' && trimmed.back() == ']') { + std::string section_name = trim(trimmed.substr(1, trimmed.size() - 2)); + std::string section_lower = section_name; + std::transform(section_lower.begin(), section_lower.end(), section_lower.begin(), + [](unsigned char c) { return std::tolower(c); }); + + if (section_lower == "pgbouncer") { + current_section = Section::PGBOUNCER; + } else if (section_lower == "databases") { + current_section = Section::DATABASES; + } else if (section_lower == "users") { + current_section = Section::USERS; + } else if (section_lower == "peers") { + current_section = Section::PEERS; + } else { + config.errors.push_back({filepath, line_num, "unknown section: " + section_name}); + current_section = Section::NONE; + ok = false; + } + continue; + } + + // Key = value line + auto eq_pos = trimmed.find('='); + if (eq_pos == std::string::npos) { + config.errors.push_back({filepath, line_num, "syntax error: expected key = value"}); + ok = false; + continue; + } + + std::string key = trim(trimmed.substr(0, eq_pos)); + std::string value = trim(trimmed.substr(eq_pos + 1)); + + // Strip inline comments from values (only for [pgbouncer] section, not connection strings) + if (current_section == Section::PGBOUNCER) { + // Remove trailing comments, but be careful with quoted values + if (!value.empty() && value[0] != '\'') { + auto comment_pos = value.find(" #"); + if (comment_pos == std::string::npos) comment_pos = value.find(" ;"); + if (comment_pos == std::string::npos) comment_pos = value.find("\t#"); + if (comment_pos == std::string::npos) comment_pos = value.find("\t;"); + if (comment_pos != std::string::npos) { + value = trim(value.substr(0, comment_pos)); + } + } + } + + if (current_section == Section::NONE) { + config.errors.push_back({filepath, line_num, "key-value pair outside of any section"}); + ok = false; + continue; + } + + switch (current_section) { + case Section::PGBOUNCER: { + if (!parse_global_key(key, value, config.global, filepath, line_num, config.errors)) { + ok = false; + } + break; + } + case Section::DATABASES: { + Database db; + if (!parse_database_entry(key, value, db, filepath, line_num, config.errors)) { + ok = false; + } else { + config.databases.push_back(std::move(db)); + } + break; + } + case Section::USERS: { + User user; + if (!parse_user_entry(key, value, user, filepath, line_num, config.errors)) { + ok = false; + } else { + config.users.push_back(std::move(user)); + } + break; + } + case Section::PEERS: { + Peer peer; + if (!parse_peer_entry(key, value, peer, filepath, line_num, config.errors)) { + ok = false; + } else { + config.peers.push_back(std::move(peer)); + } + break; + } + default: + break; + } + } + + // After parsing, resolve referenced files if requested + if (resolve_referenced_files && include_depth_ == 0) { + if (!config.global.auth_file.empty()) { + // Resolve relative path + std::string auth_path = config.global.auth_file; + if (!auth_path.empty() && auth_path[0] != '/') { + auth_path = base_dir + auth_path; + } + if (!parse_auth_file(auth_path, config.auth_entries, config.errors)) { + ok = false; + } + } + if (!config.global.auth_hba_file.empty()) { + std::string hba_path = config.global.auth_hba_file; + if (!hba_path.empty() && hba_path[0] != '/') { + hba_path = base_dir + hba_path; + } + if (!parse_hba_file(hba_path, config.hba_rules, config.errors)) { + ok = false; + } + } + } + + return ok; +} + +// --------------------------------------------------------------------------- +// Public API +// --------------------------------------------------------------------------- + +bool ConfigParser::parse( + const std::string& filepath, Config& config, + bool resolve_includes, bool resolve_referenced_files) +{ + include_depth_ = 0; + return parse_ini(filepath, config, resolve_includes, resolve_referenced_files); +} + +// --------------------------------------------------------------------------- +// Free function (declared in PgBouncer_Config.h) +// --------------------------------------------------------------------------- + +bool parse_config_file(const std::string& filepath, Config& config) { + ConfigParser parser; + return parser.parse(filepath, config); +} + +} // namespace PgBouncer diff --git a/lib/pgbouncer_compat/PgBouncer_ConfigParser.h b/lib/pgbouncer_compat/PgBouncer_ConfigParser.h new file mode 100644 index 0000000000..521f0a57a1 --- /dev/null +++ b/lib/pgbouncer_compat/PgBouncer_ConfigParser.h @@ -0,0 +1,57 @@ +#ifndef PGBOUNCER_CONFIG_PARSER_H +#define PGBOUNCER_CONFIG_PARSER_H + +#include "PgBouncer_Config.h" +#include +#include + +namespace PgBouncer { + +class ConfigParser { +public: + // Parse a pgbouncer.ini file. Returns true on success. + // On failure, errors are populated in config.errors. + // If resolve_includes is true, %include directives are followed. + // If resolve_referenced_files is true, auth_file and auth_hba_file are parsed. + bool parse(const std::string& filepath, Config& config, + bool resolve_includes = true, + bool resolve_referenced_files = true); + +private: + int include_depth_ = 0; + static const int MAX_INCLUDE_DEPTH = 10; + + bool parse_ini(const std::string& filepath, Config& config, + bool resolve_includes, bool resolve_referenced_files); + + // Section parsers + bool parse_global_key(const std::string& key, const std::string& value, + GlobalSettings& settings, const std::string& file, int line, + std::vector& errors); + bool parse_database_entry(const std::string& name, const std::string& connstr, + Database& db, const std::string& file, int line, + std::vector& errors); + bool parse_user_entry(const std::string& name, const std::string& settings_str, + User& user, const std::string& file, int line, + std::vector& errors); + bool parse_peer_entry(const std::string& name, const std::string& connstr, + Peer& peer, const std::string& file, int line, + std::vector& errors); + + // Connection string parser (key=value pairs used in [databases], [users], [peers]) + static bool parse_connstr_pairs(const std::string& connstr, + std::vector>& pairs, + const std::string& file, int line, + std::vector& errors); + + // String utilities + static std::string trim(const std::string& s); + static std::string unquote(const std::string& s); + static bool parse_bool(const std::string& value, bool& result); + static bool parse_int(const std::string& value, int& result); + static bool parse_uint(const std::string& value, unsigned int& result); +}; + +} // namespace PgBouncer + +#endif diff --git a/lib/pgbouncer_compat/PgBouncer_HBAParser.cpp b/lib/pgbouncer_compat/PgBouncer_HBAParser.cpp new file mode 100644 index 0000000000..9b6ec19b81 --- /dev/null +++ b/lib/pgbouncer_compat/PgBouncer_HBAParser.cpp @@ -0,0 +1,215 @@ +#include "PgBouncer_HBAParser.h" +#include +#include +#include +#include + +namespace PgBouncer { + +// --------------------------------------------------------------------------- +// Valid connection types and authentication methods recognised by PgBouncer +// --------------------------------------------------------------------------- + +static bool is_valid_conn_type(const std::string& t) { + return t == "local" || t == "host" || t == "hostssl" || t == "hostnossl"; +} + +static bool is_valid_method(const std::string& m) { + return m == "trust" || m == "reject" || m == "md5" || + m == "scram-sha-256" || m == "password" || m == "cert" || + m == "peer" || m == "ldap" || m == "pam"; +} + +// Return true when the token looks like an IP/CIDR (contains '/' with digits +// after it) or a bare IP address (contains '.' or ':'). Used to distinguish +// an address token from a method token when there is no CIDR suffix and the +// address is followed by a separate netmask. +static bool looks_like_address(const std::string& tok) { + if (tok == "all" || tok == "samehost" || tok == "samenet") + return true; + // Contains '/' -> CIDR notation + if (tok.find('/') != std::string::npos) + return true; + // IPv4 dotted-decimal or IPv6 colon-hex + if (tok.find('.') != std::string::npos || tok.find(':') != std::string::npos) + return true; + return false; +} + +// --------------------------------------------------------------------------- +// Tokenizer -- splits a line on whitespace, keeping double-quoted substrings +// as single tokens (quotes are stripped from the result). +// --------------------------------------------------------------------------- + +std::vector HBAParser::tokenize(const std::string& line) { + std::vector tokens; + std::string token; + bool in_quotes = false; + + for (size_t i = 0; i < line.size(); ++i) { + char c = line[i]; + + if (in_quotes) { + if (c == '"') { + in_quotes = false; + } else { + token += c; + } + } else { + if (c == '#') { + // Rest of line is a comment + break; + } else if (c == '"') { + in_quotes = true; + } else if (std::isspace(static_cast(c))) { + if (!token.empty()) { + tokens.push_back(token); + token.clear(); + } + } else { + token += c; + } + } + } + if (!token.empty()) { + tokens.push_back(token); + } + return tokens; +} + +// --------------------------------------------------------------------------- +// parse_record -- interpret one tokenized line as an HBA rule +// --------------------------------------------------------------------------- + +bool HBAParser::parse_record(const std::vector& tokens, + HBARule& rule, + const std::string& file, int lineno, + std::vector& errors) { + if (tokens.empty()) + return false; + + size_t idx = 0; + + // -- connection type -- + rule.conn_type = tokens[idx++]; + if (!is_valid_conn_type(rule.conn_type)) { + errors.push_back({file, lineno, + "invalid connection type '" + rule.conn_type + "'"}); + return false; + } + + // -- database -- + if (idx >= tokens.size()) { + errors.push_back({file, lineno, "missing database field"}); + return false; + } + rule.database = tokens[idx++]; + + // -- user -- + if (idx >= tokens.size()) { + errors.push_back({file, lineno, "missing user field"}); + return false; + } + rule.user = tokens[idx++]; + + // -- address (only for host/hostssl/hostnossl) -- + if (rule.conn_type != "local") { + if (idx >= tokens.size()) { + errors.push_back({file, lineno, "missing address field"}); + return false; + } + rule.address = tokens[idx++]; + + // If the address has no CIDR suffix and is not a keyword ("all" etc.), + // the next token might be a separate netmask rather than the method. + if (rule.address.find('/') == std::string::npos && + rule.address != "all" && + rule.address != "samehost" && + rule.address != "samenet") + { + // Peek at next token: if it looks like an IP it is a netmask + if (idx < tokens.size() && looks_like_address(tokens[idx]) && + !is_valid_method(tokens[idx])) + { + rule.mask = tokens[idx++]; + } + } + } + + // -- method -- + if (idx >= tokens.size()) { + errors.push_back({file, lineno, "missing authentication method"}); + return false; + } + rule.method = tokens[idx++]; + if (!is_valid_method(rule.method)) { + errors.push_back({file, lineno, + "invalid authentication method '" + rule.method + "'"}); + return false; + } + + // -- options (key=value pairs) -- + while (idx < tokens.size()) { + const std::string& opt = tokens[idx++]; + size_t eq = opt.find('='); + if (eq == std::string::npos) { + errors.push_back({file, lineno, + "invalid option '" + opt + "' (expected key=value)"}); + return false; + } + std::string key = opt.substr(0, eq); + std::string val = opt.substr(eq + 1); + rule.options[key] = val; + } + + return true; +} + +// --------------------------------------------------------------------------- +// parse -- read and parse an entire pg_hba.conf file +// --------------------------------------------------------------------------- + +bool HBAParser::parse(const std::string& filepath, + std::vector& rules, + std::vector& errors) { + std::ifstream in(filepath); + if (!in.is_open()) { + errors.push_back({filepath, 0, + "cannot open file '" + filepath + "'"}); + return false; + } + + std::string line; + int lineno = 0; + bool ok = true; + + while (std::getline(in, line)) { + ++lineno; + + std::vector tokens = tokenize(line); + if (tokens.empty()) + continue; + + HBARule rule; + if (parse_record(tokens, rule, filepath, lineno, errors)) { + rules.push_back(std::move(rule)); + } else { + ok = false; + } + } + + return ok; +} + +// --------------------------------------------------------------------------- +// Free function declared in PgBouncer_Config.h -- delegates to HBAParser +// --------------------------------------------------------------------------- + +bool parse_hba_file(const std::string& filepath, + std::vector& rules, + std::vector& errors) { + HBAParser parser; + return parser.parse(filepath, rules, errors); +} + +} // namespace PgBouncer diff --git a/lib/pgbouncer_compat/PgBouncer_HBAParser.h b/lib/pgbouncer_compat/PgBouncer_HBAParser.h new file mode 100644 index 0000000000..6d3fc373ed --- /dev/null +++ b/lib/pgbouncer_compat/PgBouncer_HBAParser.h @@ -0,0 +1,37 @@ +#ifndef PGBOUNCER_HBA_PARSER_H +#define PGBOUNCER_HBA_PARSER_H + +#include "PgBouncer_Config.h" +#include +#include + +namespace PgBouncer { + +class HBAParser { +public: + // Parse a pg_hba.conf file as understood by PgBouncer. + // PgBouncer supports a subset of PostgreSQL's HBA format: + // Record types: local, host, hostssl, hostnossl + // Database: all, sameuser, specific name, @file + // User: all, specific name, @file + // Address: IPv4/CIDR, IPv6/CIDR, "all" (for host/hostssl/hostnossl) + // Methods: trust, reject, md5, scram-sha-256, password, cert, peer, ldap, pam + // Options: key=value pairs after the method (e.g., map=mymap) + bool parse(const std::string& filepath, + std::vector& rules, + std::vector& errors); + +private: + // Tokenize a line respecting double-quoted strings + static std::vector tokenize(const std::string& line); + + // Parse a single HBA record from tokens + bool parse_record(const std::vector& tokens, + HBARule& rule, + const std::string& file, int line, + std::vector& errors); +}; + +} // namespace PgBouncer + +#endif diff --git a/test/tap/tests/unit/Makefile b/test/tap/tests/unit/Makefile index cc24062b76..ebe1184a70 100644 --- a/test/tap/tests/unit/Makefile +++ b/test/tap/tests/unit/Makefile @@ -452,7 +452,8 @@ UNIT_TESTS := smoke_test-t query_cache_unit-t query_processor_unit-t \ pgsql_response_framer_traffic_unit-t \ ffto_state_machine_unit-t \ restapi_server_unit-t \ - mcp_client_unit-t + mcp_client_unit-t \ + pgbouncer_config_parser_unit-t ifeq ($(PROXYSQL31),1) UNIT_TESTS += caching_sha2_rsa_unit-t @@ -853,6 +854,17 @@ ifeq ($(UNAME_S),Linux) caching_sha2_rsa_unit-t: ALLOW_MULTI_DEF += -Wl,--wrap=flock endif +# PgBouncer config parser: standalone test (no libproxysql.a needed) +PGBOUNCER_COMPAT_SRCS := $(PROXYSQL_PATH)/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp \ + $(PROXYSQL_PATH)/lib/pgbouncer_compat/PgBouncer_AuthFileParser.cpp \ + $(PROXYSQL_PATH)/lib/pgbouncer_compat/PgBouncer_HBAParser.cpp + +pgbouncer_config_parser_unit-t: pgbouncer_config_parser_unit-t.cpp $(ODIR)/tap.o $(ODIR)/tap_noise_stubs.o $(PGBOUNCER_COMPAT_SRCS) + $(CXX) $< $(PGBOUNCER_COMPAT_SRCS) $(ODIR)/tap.o $(ODIR)/tap_noise_stubs.o \ + -I$(TAP_IDIR) -I$(PROXYSQL_PATH)/include \ + -I$(PROXYSQL_PATH)/lib/pgbouncer_compat \ + $(STDCPP) -O0 -ggdb $(WGCOV) $(LWGCOV) -lpthread -o $@ + # Pattern rule: all unit tests use the same compile + link flags. # Each test binary is built from its .cpp source, linked against # the test harness objects and libproxysql.a with all dependencies. diff --git a/test/tap/tests/unit/fixtures/pgbouncer_compat/full.ini b/test/tap/tests/unit/fixtures/pgbouncer_compat/full.ini new file mode 100644 index 0000000000..4829095793 --- /dev/null +++ b/test/tap/tests/unit/fixtures/pgbouncer_compat/full.ini @@ -0,0 +1,52 @@ +[pgbouncer] +listen_addr = 0.0.0.0 +listen_port = 6432 +unix_socket_dir = /var/run/postgresql + +auth_type = md5 +auth_file = userlist.txt + +pool_mode = transaction +default_pool_size = 25 +min_pool_size = 5 +reserve_pool_size = 5 +max_client_conn = 1000 +max_db_connections = 100 +max_user_connections = 50 + +server_lifetime = 3600 +server_idle_timeout = 600 +server_connect_timeout = 15 +query_timeout = 30 +idle_transaction_timeout = 10 +client_idle_timeout = 300 + +server_tls_sslmode = require +server_tls_ca_file = /etc/ssl/certs/ca.pem + +log_connections = 1 +log_disconnections = 1 +verbose = 2 +stats_period = 30 + +admin_users = admin +tcp_keepalive = 1 +tcp_keepidle = 60 + +max_prepared_statements = 100 + +[databases] +prod = host=db1.example.com port=5432 dbname=production pool_size=20 pool_mode=session +multi = host=db1,db2,db3 dbname=shared load_balance_hosts=round-robin +authdb = host=authsrv.com dbname=postgres auth_user=authuser +withquery = host=db2.example.com connect_query='SET statement_timeout = 5000' +* = host=default.example.com + +[users] +appuser = pool_mode=transaction pool_size=10 max_user_connections=50 +admin = pool_mode=session max_user_connections=5 +readonly = query_timeout=60 idle_transaction_timeout=30 + +[peers] +1 = host=pgbouncer1.example.com port=6432 +2 = host=pgbouncer2.example.com port=6433 pool_size=3 diff --git a/test/tap/tests/unit/fixtures/pgbouncer_compat/include_databases.ini b/test/tap/tests/unit/fixtures/pgbouncer_compat/include_databases.ini new file mode 100644 index 0000000000..e5a2bac128 --- /dev/null +++ b/test/tap/tests/unit/fixtures/pgbouncer_compat/include_databases.ini @@ -0,0 +1,2 @@ +[databases] +included_db = host=included.example.com port=5433 diff --git a/test/tap/tests/unit/fixtures/pgbouncer_compat/include_main.ini b/test/tap/tests/unit/fixtures/pgbouncer_compat/include_main.ini new file mode 100644 index 0000000000..e700124576 --- /dev/null +++ b/test/tap/tests/unit/fixtures/pgbouncer_compat/include_main.ini @@ -0,0 +1,5 @@ +[pgbouncer] +listen_port = 6432 +auth_type = md5 + +%include include_databases.ini diff --git a/test/tap/tests/unit/fixtures/pgbouncer_compat/malformed.ini b/test/tap/tests/unit/fixtures/pgbouncer_compat/malformed.ini new file mode 100644 index 0000000000..2b3e1cca93 --- /dev/null +++ b/test/tap/tests/unit/fixtures/pgbouncer_compat/malformed.ini @@ -0,0 +1,9 @@ +[pgbouncer] +listen_port = 6432 +unknown_param = something + +[databases] +mydb = host=localhost + +[bogus_section] +foo = bar diff --git a/test/tap/tests/unit/fixtures/pgbouncer_compat/minimal.ini b/test/tap/tests/unit/fixtures/pgbouncer_compat/minimal.ini new file mode 100644 index 0000000000..d018ee7c49 --- /dev/null +++ b/test/tap/tests/unit/fixtures/pgbouncer_compat/minimal.ini @@ -0,0 +1,6 @@ +[pgbouncer] +listen_port = 6432 +auth_type = trust + +[databases] +mydb = host=localhost diff --git a/test/tap/tests/unit/fixtures/pgbouncer_compat/pg_hba.conf b/test/tap/tests/unit/fixtures/pgbouncer_compat/pg_hba.conf new file mode 100644 index 0000000000..e00db92b07 --- /dev/null +++ b/test/tap/tests/unit/fixtures/pgbouncer_compat/pg_hba.conf @@ -0,0 +1,6 @@ +# TYPE DATABASE USER ADDRESS METHOD +local all all peer +host all all 127.0.0.1/32 md5 +host all all ::1/128 scram-sha-256 +hostssl mydb admin 10.0.0.0/8 cert map=mymap +hostnossl all all 0.0.0.0/0 reject diff --git a/test/tap/tests/unit/fixtures/pgbouncer_compat/userlist.txt b/test/tap/tests/unit/fixtures/pgbouncer_compat/userlist.txt new file mode 100644 index 0000000000..c6f6d236d0 --- /dev/null +++ b/test/tap/tests/unit/fixtures/pgbouncer_compat/userlist.txt @@ -0,0 +1,4 @@ +"appuser" "secretpassword" +"admin" "md5abcdef0123456789abcdef0123456789" +"scramuser" "SCRAM-SHA-256$4096:c2FsdA==$storedkey:serverkey" +"quoted""user" "pass""word" diff --git a/test/tap/tests/unit/pgbouncer_config_parser_unit-t.cpp b/test/tap/tests/unit/pgbouncer_config_parser_unit-t.cpp new file mode 100644 index 0000000000..c682730330 --- /dev/null +++ b/test/tap/tests/unit/pgbouncer_config_parser_unit-t.cpp @@ -0,0 +1,364 @@ +/** + * @file pgbouncer_config_parser_unit-t.cpp + * @brief Unit tests for PgBouncer configuration file parser. + * + * Tests the parser library in isolation — no ProxySQL dependencies. + * Covers: INI parsing, auth file parsing, HBA parsing, %include, edge cases. + */ + +#include "tap.h" +#include "PgBouncer_Config.h" + +// Test helpers +#define CHECK(cond, msg) ok((cond), "%s", (msg)) +#define CHECK_STR(actual, expected, msg) \ + ok((actual) == (expected), "%s: got '%s', expected '%s'", (msg), (actual).c_str(), (expected)) +#define CHECK_INT(actual, expected, msg) \ + ok((actual) == (expected), "%s: got %d, expected %d", (msg), (actual), (expected)) + +// ============================================================ +// Test: Minimal config parsing +// ============================================================ +void test_minimal_config() { + PgBouncer::Config config; + bool ok_result = PgBouncer::parse_config_file( + "fixtures/pgbouncer_compat/minimal.ini", config); + + CHECK(ok_result, "minimal config parses successfully"); + CHECK(config.errors.empty(), "minimal config has no errors"); + CHECK_INT(config.global.listen_port, 6432, "listen_port"); + CHECK_STR(config.global.auth_type, "trust", "auth_type"); + CHECK_INT((int)config.databases.size(), 1, "one database entry"); + CHECK_STR(config.databases[0].name, "mydb", "database name"); + CHECK_STR(config.databases[0].host, "localhost", "database host"); + CHECK_INT(config.databases[0].port, 5432, "database port (default)"); +} + +// ============================================================ +// Test: Full config parsing +// ============================================================ +void test_full_config() { + PgBouncer::Config config; + bool ok_result = PgBouncer::parse_config_file( + "fixtures/pgbouncer_compat/full.ini", config); + + CHECK(ok_result, "full config parses successfully"); + + // Global settings + CHECK_STR(config.global.listen_addr, "0.0.0.0", "listen_addr"); + CHECK_INT(config.global.listen_port, 6432, "listen_port"); + CHECK_STR(config.global.unix_socket_dir, "/var/run/postgresql", "unix_socket_dir"); + CHECK_STR(config.global.auth_type, "md5", "auth_type"); + CHECK_STR(config.global.pool_mode, "transaction", "pool_mode"); + CHECK_INT(config.global.default_pool_size, 25, "default_pool_size"); + CHECK_INT(config.global.min_pool_size, 5, "min_pool_size"); + CHECK_INT(config.global.reserve_pool_size, 5, "reserve_pool_size"); + CHECK_INT(config.global.max_client_conn, 1000, "max_client_conn"); + CHECK_INT(config.global.max_db_connections, 100, "max_db_connections"); + CHECK_INT(config.global.max_user_connections, 50, "max_user_connections"); + CHECK_INT(config.global.server_lifetime, 3600, "server_lifetime"); + CHECK_INT(config.global.server_idle_timeout, 600, "server_idle_timeout"); + CHECK_INT(config.global.server_connect_timeout, 15, "server_connect_timeout"); + CHECK_INT(config.global.query_timeout, 30, "query_timeout"); + CHECK_INT(config.global.idle_transaction_timeout, 10, "idle_transaction_timeout"); + CHECK_INT(config.global.client_idle_timeout, 300, "client_idle_timeout"); + CHECK_STR(config.global.server_tls_sslmode, "require", "server_tls_sslmode"); + CHECK_STR(config.global.server_tls_ca_file, "/etc/ssl/certs/ca.pem", "server_tls_ca_file"); + CHECK(config.global.log_connections, "log_connections"); + CHECK(config.global.log_disconnections, "log_disconnections"); + CHECK_INT(config.global.verbose, 2, "verbose"); + CHECK_INT(config.global.stats_period, 30, "stats_period"); + CHECK_STR(config.global.admin_users, "admin", "admin_users"); + CHECK(config.global.tcp_keepalive, "tcp_keepalive"); + CHECK_INT(config.global.tcp_keepidle, 60, "tcp_keepidle"); + CHECK_INT(config.global.max_prepared_statements, 100, "max_prepared_statements"); + + // Databases + CHECK_INT((int)config.databases.size(), 5, "five database entries"); + + // prod database + bool found_prod = false; + for (const auto& db : config.databases) { + if (db.name == "prod") { + found_prod = true; + CHECK_STR(db.host, "db1.example.com", "prod host"); + CHECK_INT(db.port, 5432, "prod port"); + CHECK_STR(db.dbname, "production", "prod dbname"); + CHECK_INT(db.pool_size, 20, "prod pool_size"); + CHECK_STR(db.pool_mode, "session", "prod pool_mode"); + } + } + CHECK(found_prod, "prod database found"); + + // multi-host database + bool found_multi = false; + for (const auto& db : config.databases) { + if (db.name == "multi") { + found_multi = true; + CHECK_STR(db.host, "db1,db2,db3", "multi host (comma-separated)"); + CHECK_STR(db.dbname, "shared", "multi dbname"); + CHECK_STR(db.load_balance_hosts, "round-robin", "multi load_balance_hosts"); + } + } + CHECK(found_multi, "multi database found"); + + // wildcard database + bool found_wildcard = false; + for (const auto& db : config.databases) { + if (db.name == "*") { + found_wildcard = true; + CHECK_STR(db.host, "default.example.com", "wildcard host"); + } + } + CHECK(found_wildcard, "wildcard database found"); + + // connect_query with single-quoted value + bool found_withquery = false; + for (const auto& db : config.databases) { + if (db.name == "withquery") { + found_withquery = true; + CHECK_STR(db.connect_query, "SET statement_timeout = 5000", "connect_query value"); + } + } + CHECK(found_withquery, "withquery database found"); + + // Users + CHECK_INT((int)config.users.size(), 3, "three user entries"); + + bool found_appuser = false; + for (const auto& u : config.users) { + if (u.name == "appuser") { + found_appuser = true; + CHECK_STR(u.pool_mode, "transaction", "appuser pool_mode"); + CHECK_INT(u.pool_size, 10, "appuser pool_size"); + CHECK_INT(u.max_user_connections, 50, "appuser max_user_connections"); + } + } + CHECK(found_appuser, "appuser found"); + + bool found_readonly = false; + for (const auto& u : config.users) { + if (u.name == "readonly") { + found_readonly = true; + CHECK_INT(u.query_timeout, 60, "readonly query_timeout"); + CHECK_INT(u.idle_transaction_timeout, 30, "readonly idle_transaction_timeout"); + } + } + CHECK(found_readonly, "readonly user found"); + + // Peers + CHECK_INT((int)config.peers.size(), 2, "two peer entries"); + + bool found_peer2 = false; + for (const auto& p : config.peers) { + if (p.peer_id == 2) { + found_peer2 = true; + CHECK_STR(p.host, "pgbouncer2.example.com", "peer 2 host"); + CHECK_INT(p.port, 6433, "peer 2 port"); + CHECK_INT(p.pool_size, 3, "peer 2 pool_size"); + } + } + CHECK(found_peer2, "peer 2 found"); + + // Auth file entries (resolved from auth_file) + CHECK_INT((int)config.auth_entries.size(), 4, "four auth entries"); + + bool found_auth_plain = false; + bool found_auth_md5 = false; + bool found_auth_scram = false; + for (const auto& a : config.auth_entries) { + if (a.username == "appuser") { + found_auth_plain = true; + CHECK(a.type == PgBouncer::AuthType::PLAIN, "appuser is PLAIN auth"); + CHECK_STR(a.password, "secretpassword", "appuser password"); + } + if (a.username == "admin") { + found_auth_md5 = true; + CHECK(a.type == PgBouncer::AuthType::MD5, "admin is MD5 auth"); + } + if (a.username == "scramuser") { + found_auth_scram = true; + CHECK(a.type == PgBouncer::AuthType::SCRAM, "scramuser is SCRAM auth"); + } + } + CHECK(found_auth_plain, "plain auth entry found"); + CHECK(found_auth_md5, "md5 auth entry found"); + CHECK(found_auth_scram, "scram auth entry found"); +} + +// ============================================================ +// Test: Auth file parsing standalone +// ============================================================ +void test_auth_file() { + std::vector entries; + std::vector errors; + bool ok_result = PgBouncer::parse_auth_file( + "fixtures/pgbouncer_compat/userlist.txt", entries, errors); + + CHECK(ok_result, "auth file parses successfully"); + CHECK_INT((int)entries.size(), 4, "four entries in auth file"); + + // Plain password + CHECK_STR(entries[0].username, "appuser", "entry 0 username"); + CHECK_STR(entries[0].password, "secretpassword", "entry 0 password"); + CHECK(entries[0].type == PgBouncer::AuthType::PLAIN, "entry 0 is PLAIN"); + + // MD5 password + CHECK_STR(entries[1].username, "admin", "entry 1 username"); + CHECK(entries[1].type == PgBouncer::AuthType::MD5, "entry 1 is MD5"); + + // SCRAM password + CHECK_STR(entries[2].username, "scramuser", "entry 2 username"); + CHECK(entries[2].type == PgBouncer::AuthType::SCRAM, "entry 2 is SCRAM"); + + // Quoted username with escaped quotes + CHECK_STR(entries[3].username, "quoted\"user", "entry 3 username with escaped quote"); + CHECK_STR(entries[3].password, "pass\"word", "entry 3 password with escaped quote"); +} + +// ============================================================ +// Test: HBA file parsing +// ============================================================ +void test_hba_file() { + std::vector rules; + std::vector errors; + bool ok_result = PgBouncer::parse_hba_file( + "fixtures/pgbouncer_compat/pg_hba.conf", rules, errors); + + CHECK(ok_result, "hba file parses successfully"); + CHECK(errors.empty(), "hba file has no errors"); + CHECK_INT((int)rules.size(), 5, "five HBA rules"); + + // local rule + CHECK_STR(rules[0].conn_type, "local", "rule 0 type"); + CHECK_STR(rules[0].database, "all", "rule 0 database"); + CHECK_STR(rules[0].user, "all", "rule 0 user"); + CHECK_STR(rules[0].method, "peer", "rule 0 method"); + + // host with IPv4 + CHECK_STR(rules[1].conn_type, "host", "rule 1 type"); + CHECK_STR(rules[1].address, "127.0.0.1/32", "rule 1 address"); + CHECK_STR(rules[1].method, "md5", "rule 1 method"); + + // host with IPv6 + CHECK_STR(rules[2].conn_type, "host", "rule 2 type"); + CHECK_STR(rules[2].address, "::1/128", "rule 2 address"); + CHECK_STR(rules[2].method, "scram-sha-256", "rule 2 method"); + + // hostssl with options + CHECK_STR(rules[3].conn_type, "hostssl", "rule 3 type"); + CHECK_STR(rules[3].database, "mydb", "rule 3 database"); + CHECK_STR(rules[3].user, "admin", "rule 3 user"); + CHECK_STR(rules[3].address, "10.0.0.0/8", "rule 3 address"); + CHECK_STR(rules[3].method, "cert", "rule 3 method"); + CHECK(rules[3].options.count("map") > 0, "rule 3 has map option"); + CHECK_STR(rules[3].options.at("map"), "mymap", "rule 3 map=mymap"); + + // hostnossl reject + CHECK_STR(rules[4].conn_type, "hostnossl", "rule 4 type"); + CHECK_STR(rules[4].method, "reject", "rule 4 method"); +} + +// ============================================================ +// Test: Malformed config (strict parsing) +// ============================================================ +void test_malformed_config() { + PgBouncer::Config config; + bool ok_result = PgBouncer::parse_config_file( + "fixtures/pgbouncer_compat/malformed.ini", config); + + CHECK(!ok_result, "malformed config fails to parse"); + CHECK(!config.errors.empty(), "malformed config has errors"); + + // Should have errors for unknown_param and bogus_section + bool found_unknown_param = false; + bool found_unknown_section = false; + for (const auto& err : config.errors) { + if (err.message.find("unknown_param") != std::string::npos || + err.message.find("Unknown") != std::string::npos) { + found_unknown_param = true; + } + if (err.message.find("bogus_section") != std::string::npos || + err.message.find("unknown section") != std::string::npos || + err.message.find("Unknown section") != std::string::npos) { + found_unknown_section = true; + } + } + CHECK(found_unknown_param || found_unknown_section, + "errors mention unknown parameter or section"); +} + +// ============================================================ +// Test: %include directive +// ============================================================ +void test_include_directive() { + PgBouncer::Config config; + bool ok_result = PgBouncer::parse_config_file( + "fixtures/pgbouncer_compat/include_main.ini", config); + + CHECK(ok_result, "include config parses successfully"); + CHECK_INT(config.global.listen_port, 6432, "listen_port from main"); + CHECK_STR(config.global.auth_type, "md5", "auth_type from main"); + CHECK_INT((int)config.databases.size(), 1, "one database from included file"); + if (!config.databases.empty()) { + CHECK_STR(config.databases[0].name, "included_db", "included database name"); + CHECK_STR(config.databases[0].host, "included.example.com", "included database host"); + CHECK_INT(config.databases[0].port, 5433, "included database port"); + } else { + ok(0, "included database name - SKIPPED (no databases)"); + ok(0, "included database host - SKIPPED (no databases)"); + ok(0, "included database port - SKIPPED (no databases)"); + } +} + +// ============================================================ +// Test: Nonexistent file +// ============================================================ +void test_nonexistent_file() { + PgBouncer::Config config; + bool ok_result = PgBouncer::parse_config_file( + "fixtures/pgbouncer_compat/does_not_exist.ini", config); + + CHECK(!ok_result, "nonexistent file returns false"); + CHECK(!config.errors.empty(), "nonexistent file produces errors"); +} + +// ============================================================ +// Test: Default values +// ============================================================ +void test_defaults() { + PgBouncer::Config config; + bool ok_result = PgBouncer::parse_config_file( + "fixtures/pgbouncer_compat/minimal.ini", config); + + CHECK(ok_result, "minimal config parses for defaults test"); + + // Verify defaults that were NOT explicitly set in minimal.ini + CHECK_STR(config.global.pool_mode, "session", "default pool_mode is session"); + CHECK_INT(config.global.default_pool_size, 20, "default pool_size is 20"); + CHECK_INT(config.global.max_client_conn, 100, "default max_client_conn is 100"); + CHECK_INT(config.global.server_lifetime, 3600, "default server_lifetime is 3600"); + CHECK_INT(config.global.server_idle_timeout, 600, "default server_idle_timeout is 600"); + CHECK_STR(config.global.server_reset_query, "DISCARD ALL", "default server_reset_query"); + CHECK(!config.global.syslog, "default syslog is false"); + CHECK(config.global.tcp_keepalive, "default tcp_keepalive is true"); + CHECK_INT(config.global.max_prepared_statements, 200, "default max_prepared_statements is 200"); +} + +// ============================================================ +// Main +// ============================================================ +int main() { + plan(127); + + test_minimal_config(); // 7 tests + test_full_config(); // 42 tests + test_auth_file(); // 11 tests + test_hba_file(); // 19 tests + test_malformed_config(); // 3 tests + test_include_directive(); // 7 tests + test_nonexistent_file(); // 2 tests + test_defaults(); // 9 tests (adjusted: removed 1 duplicate) + + return exit_status(); +} From 55e617b0694c7186947eddc1b3a830fc4a2113f9 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 3 Apr 2026 13:25:05 +0000 Subject: [PATCH 02/12] feat: add PgBouncer config converter, CLI tool, and SHOW commands (Stages 2+3) (#5564, #5565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 2 - Config Converter + CLI: - PgBouncer_ConfigConverter: Converts parsed PgBouncer config into ProxySQL SQL statements with full parameter mapping - Databases → pgsql_servers + pgsql_query_rules (auto hostgroup assignment) - Multi-host entries → multiple server rows with equal weight - Users → pgsql_users with pool mode mapping (session→fast_forward, transaction→transaction_persistent, statement→defaults) - Globals → pgsql-* variables with unit conversion (s→ms) - HBA rules → pgsql_firewall_whitelist entries - Strict mode (default): fails on unmappable parameters - Relaxed mode (--ignore-warnings): warns and continues - Dry-run output: commented SQL with summary - ProxySQL_CLI: Entry point for proxysql-cli binary (argv[0] detection) - Subcommand: import-pgbouncer [--dry-run] [--ignore-warnings] - Never starts daemon, always exits after command - 39 unit tests covering: minimal conversion, multi-host, wildcard database, pool mode mapping, global settings, strict/relaxed modes, query rules, dry-run format, TLS settings Stage 3 - PgBouncer-Compatible SHOW Commands: - PgBouncer_ShowCommands: Translates PgBouncer SHOW commands to equivalent ProxySQL SQL queries with exact column output format - SHOW POOLS, STATS, SERVERS, CLIENTS, DATABASES, USERS, CONFIG, VERSION, STATE, LISTS - SHOW EXTENDED variant adds ProxySQL-specific columns - Unsupported commands (DNS_HOSTS, PEERS, etc.) return clear errors - Case-insensitive, handles trailing semicolons and extra whitespace - 39 unit tests covering: command recognition, case insensitivity, trailing semicolons, EXTENDED variant, SQL output columns, non-matching queries, unsupported commands --- lib/Makefile | 14 +- .../PgBouncer_ConfigConverter.cpp | 651 ++++++++++++++++++ .../PgBouncer_ConfigConverter.h | 64 ++ .../PgBouncer_ShowCommands.cpp | 355 ++++++++++ lib/pgbouncer_compat/PgBouncer_ShowCommands.h | 22 + lib/pgbouncer_compat/ProxySQL_CLI.cpp | 140 ++++ lib/pgbouncer_compat/ProxySQL_CLI.h | 9 + test/tap/tests/unit/Makefile | 20 +- .../tests/unit/pgbouncer_converter_unit-t.cpp | 293 ++++++++ .../unit/pgbouncer_show_commands_unit-t.cpp | 204 ++++++ 10 files changed, 1770 insertions(+), 2 deletions(-) create mode 100644 lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp create mode 100644 lib/pgbouncer_compat/PgBouncer_ConfigConverter.h create mode 100644 lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp create mode 100644 lib/pgbouncer_compat/PgBouncer_ShowCommands.h create mode 100644 lib/pgbouncer_compat/ProxySQL_CLI.cpp create mode 100644 lib/pgbouncer_compat/ProxySQL_CLI.h create mode 100644 test/tap/tests/unit/pgbouncer_converter_unit-t.cpp create mode 100644 test/tap/tests/unit/pgbouncer_show_commands_unit-t.cpp diff --git a/lib/Makefile b/lib/Makefile index 8d8e93dd10..b9c0296bf3 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -127,7 +127,10 @@ _OBJ_CXX := ProxySQL_GloVars.oo network.oo debug.oo configfile.oo Query_Cache.oo proxy_sqlite3_symbols.oo \ PgBouncer_ConfigParser.oo \ PgBouncer_AuthFileParser.oo \ - PgBouncer_HBAParser.oo + PgBouncer_HBAParser.oo \ + PgBouncer_ConfigConverter.oo \ + PgBouncer_ShowCommands.oo \ + ProxySQL_CLI.oo ifeq ($(PROXYSQL31),1) _OBJ_CXX += MySQL_Caching_Sha2_RSA.oo @@ -167,6 +170,15 @@ $(ODIR)/PgBouncer_AuthFileParser.oo: pgbouncer_compat/PgBouncer_AuthFileParser.c $(ODIR)/PgBouncer_HBAParser.oo: pgbouncer_compat/PgBouncer_HBAParser.cpp $(HEADERS) $(CXX) -fPIC -c -o $@ $< $(MYCXXFLAGS) $(CXXFLAGS) -Ipgbouncer_compat +$(ODIR)/PgBouncer_ConfigConverter.oo: pgbouncer_compat/PgBouncer_ConfigConverter.cpp $(HEADERS) + $(CXX) -fPIC -c -o $@ $< $(MYCXXFLAGS) $(CXXFLAGS) -Ipgbouncer_compat + +$(ODIR)/PgBouncer_ShowCommands.oo: pgbouncer_compat/PgBouncer_ShowCommands.cpp $(HEADERS) + $(CXX) -fPIC -c -o $@ $< $(MYCXXFLAGS) $(CXXFLAGS) -Ipgbouncer_compat + +$(ODIR)/ProxySQL_CLI.oo: pgbouncer_compat/ProxySQL_CLI.cpp $(HEADERS) + $(CXX) -fPIC -c -o $@ $< $(MYCXXFLAGS) $(CXXFLAGS) -Ipgbouncer_compat + $(ODIR)/%.oo: %.cpp $(HEADERS) $(CXX) -fPIC -c -o $@ $< $(MYCXXFLAGS) $(CXXFLAGS) diff --git a/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp b/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp new file mode 100644 index 0000000000..57b92005f1 --- /dev/null +++ b/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp @@ -0,0 +1,651 @@ +#include "PgBouncer_ConfigConverter.h" +#include +#include +#include + +namespace PgBouncer { + +// --------------------------------------------------------------------------- +// sql_escape: double single-quotes for SQL string literals +// --------------------------------------------------------------------------- +std::string ConfigConverter::sql_escape(const std::string& s) { + std::string out; + out.reserve(s.size() + 8); + for (char c : s) { + if (c == '\'') out += "''"; + else out += c; + } + return out; +} + +// --------------------------------------------------------------------------- +// add_issue: error (strict) or warning (relaxed) +// --------------------------------------------------------------------------- +void ConfigConverter::add_issue(ConversionResult& result, bool strict, + const std::string& msg) { + ParseMessage pm; + pm.message = msg; + if (strict) { + result.errors.push_back(pm); + result.success = false; + } else { + result.warnings.push_back(pm); + } +} + +// --------------------------------------------------------------------------- +// convert (top-level entry point) +// --------------------------------------------------------------------------- +ConversionResult ConfigConverter::convert(const Config& config, bool strict) { + // Reset state + next_hostgroup_ = 0; + next_rule_id_ = 1; + wildcard_hostgroup_ = -1; + + ConversionResult result; + + convert_databases(config, result); + convert_users(config, result); + convert_globals(config, result, strict); + convert_hba_rules(config, result, strict); + check_unmappable(config, result, strict); + add_load_and_save(result); + + return result; +} + +// --------------------------------------------------------------------------- +// Helper: split a string on a delimiter +// --------------------------------------------------------------------------- +static std::vector split(const std::string& s, char delim) { + std::vector parts; + std::istringstream ss(s); + std::string token; + while (std::getline(ss, token, delim)) { + // trim whitespace + size_t start = token.find_first_not_of(" \t"); + size_t end = token.find_last_not_of(" \t"); + if (start != std::string::npos) + parts.push_back(token.substr(start, end - start + 1)); + } + return parts; +} + +// --------------------------------------------------------------------------- +// convert_databases +// --------------------------------------------------------------------------- +void ConfigConverter::convert_databases(const Config& config, + ConversionResult& result) { + if (config.databases.empty()) return; + + // Clean slate + result.entries.push_back({ + "DELETE FROM pgsql_servers;", + "Remove existing server entries before importing" + }); + result.entries.push_back({ + "DELETE FROM pgsql_query_rules;", + "Remove existing query rules before importing" + }); + + for (const auto& db : config.databases) { + int hg = next_hostgroup_++; + + // Wildcard database: remember its hostgroup for user defaults + if (db.name == "*") { + wildcard_hostgroup_ = hg; + } + + // Resolve host list + std::string host_str = db.host.empty() ? "127.0.0.1" : db.host; + std::vector hosts = split(host_str, ','); + int port = db.port; + + // max_connections per server from pool_size (or default 20) + int max_conn = (db.pool_size > 0) ? db.pool_size : 20; + + // Weight: equal across all hosts + int weight = 1000; + + bool use_ssl = false; + // SSL will be set later in convert_globals if server_tls_sslmode requires it + + for (const auto& h : hosts) { + std::ostringstream sql; + sql << "INSERT INTO pgsql_servers " + << "(hostgroup_id, hostname, port, max_connections, weight, use_ssl) " + << "VALUES (" + << hg << ", " + << "'" << sql_escape(h) << "', " + << port << ", " + << max_conn << ", " + << weight << ", " + << (use_ssl ? 1 : 0) + << ");"; + + std::string comment = "Server for database '" + db.name + "'"; + if (hosts.size() > 1) + comment += " (multi-host: " + host_str + ")"; + + result.entries.push_back({sql.str(), comment}); + result.server_count++; + } + + // Query rule to route by database name (skip for wildcard) + if (db.name != "*") { + int rule_id = next_rule_id_++; + + std::string dest_db = db.dbname.empty() ? db.name : db.dbname; + + std::ostringstream sql; + sql << "INSERT INTO pgsql_query_rules " + << "(rule_id, active, schemaname, destination_hostgroup, apply) " + << "VALUES (" + << rule_id << ", 1, " + << "'" << sql_escape(db.name) << "', " + << hg << ", 1);"; + + std::string comment = "Route database '" + db.name + "' to hostgroup " + std::to_string(hg); + if (db.dbname != "" && db.dbname != db.name) + comment += " (backend db: " + db.dbname + ")"; + + result.entries.push_back({sql.str(), comment}); + result.rule_count++; + } + } +} + +// --------------------------------------------------------------------------- +// convert_users +// --------------------------------------------------------------------------- +void ConfigConverter::convert_users(const Config& config, + ConversionResult& result) { + // Build a password lookup from auth_entries + std::map passwords; + for (const auto& ae : config.auth_entries) { + passwords[ae.username] = ae.password; + } + + // Collect users from [users] section; also add any auth_entries users not + // already listed. + std::vector user_list = config.users; + std::map seen; + for (const auto& u : user_list) seen[u.name] = true; + for (const auto& ae : config.auth_entries) { + if (!seen[ae.username]) { + User u; + u.name = ae.username; + user_list.push_back(u); + seen[ae.username] = true; + } + } + + if (user_list.empty()) return; + + // Clean slate + result.entries.push_back({ + "DELETE FROM pgsql_users;", + "Remove existing user entries before importing" + }); + + int default_hg = (wildcard_hostgroup_ >= 0) ? wildcard_hostgroup_ : 0; + + for (const auto& u : user_list) { + // Resolve password from auth_entries + std::string password; + auto it = passwords.find(u.name); + if (it != passwords.end()) password = it->second; + + // Pool mode mapping + std::string pool = u.pool_mode.empty() ? config.global.pool_mode : u.pool_mode; + int fast_forward = 0; + int transaction_persistent = 0; + if (pool == "session") { + fast_forward = 1; + } else if (pool == "transaction") { + transaction_persistent = 1; + } + // statement mode and default: no special flags + + // max_connections from max_user_connections + int max_conn = (u.max_user_connections > 0) ? u.max_user_connections : 0; + + std::ostringstream sql; + sql << "INSERT INTO pgsql_users " + << "(username, password, default_hostgroup, max_connections, " + << "fast_forward, transaction_persistent, active, backend, frontend) " + << "VALUES (" + << "'" << sql_escape(u.name) << "', " + << "'" << sql_escape(password) << "', " + << default_hg << ", " + << max_conn << ", " + << fast_forward << ", " + << transaction_persistent << ", " + << "1, 1, 1);"; + + std::string comment = "User '" + u.name + "'"; + if (fast_forward) + comment += " (session mode -> fast_forward)"; + else if (transaction_persistent) + comment += " (transaction mode -> transaction_persistent)"; + + result.entries.push_back({sql.str(), comment}); + result.user_count++; + } +} + +// --------------------------------------------------------------------------- +// convert_globals +// --------------------------------------------------------------------------- +void ConfigConverter::convert_globals(const Config& config, + ConversionResult& result, bool strict) { + const auto& g = config.global; + + // Helper lambda to emit a SET + track variable count + auto emit_set = [&](const std::string& var, const std::string& val, + const std::string& comment) { + std::ostringstream sql; + sql << "SET " << var << "='" << sql_escape(val) << "';"; + result.entries.push_back({sql.str(), comment}); + result.variable_count++; + }; + + auto emit_set_int = [&](const std::string& var, int val, + const std::string& comment) { + std::ostringstream sql; + sql << "SET " << var << "=" << val << ";"; + result.entries.push_back({sql.str(), comment}); + result.variable_count++; + }; + + // -- listen_addr:listen_port -> pgsql-interfaces + if (!g.listen_addr.empty()) { + std::string iface = g.listen_addr + ":" + std::to_string(g.listen_port); + emit_set("pgsql-interfaces", iface, + "PgBouncer listen_addr:listen_port -> ProxySQL pgsql-interfaces"); + } + + // -- max_client_conn -> pgsql-max_connections (ProxySQL default: 2048) + if (g.max_client_conn != 100) { + // PgBouncer default is 100; only emit if changed + emit_set_int("pgsql-max_connections", g.max_client_conn, + "PgBouncer max_client_conn -> ProxySQL pgsql-max_connections"); + } + + // -- server_connect_timeout (seconds -> milliseconds, ProxySQL default: 10000) + if (g.server_connect_timeout != 15) { + emit_set_int("pgsql-connect_timeout_server", g.server_connect_timeout * 1000, + "PgBouncer server_connect_timeout (s) -> ProxySQL (ms)"); + } + + // -- server_idle_timeout (seconds -> milliseconds, ProxySQL default: 0) + if (g.server_idle_timeout != 600) { + emit_set_int("pgsql-connection_max_age_ms", g.server_idle_timeout * 1000, + "PgBouncer server_idle_timeout (s) -> ProxySQL connection_max_age_ms (ms)"); + } + + // -- client_idle_timeout (seconds -> milliseconds) + if (g.client_idle_timeout != 0) { + emit_set_int("pgsql-wait_timeout", g.client_idle_timeout * 1000, + "PgBouncer client_idle_timeout (s) -> ProxySQL wait_timeout (ms)"); + } + + // -- query_timeout (seconds -> milliseconds) + if (g.query_timeout != 0) { + emit_set_int("pgsql-long_query_time", g.query_timeout * 1000, + "PgBouncer query_timeout (s) -> ProxySQL long_query_time (ms). " + "Note: ProxySQL logs long queries but does not kill them by default; " + "consider adding a query rule with timeout to replicate kill behavior"); + } + + // -- idle_transaction_timeout (seconds -> milliseconds) + if (g.idle_transaction_timeout != 0) { + emit_set_int("pgsql-max_transaction_idle_time", g.idle_transaction_timeout * 1000, + "PgBouncer idle_transaction_timeout (s) -> ProxySQL (ms)"); + } + + // -- transaction_timeout (seconds -> milliseconds) + if (g.transaction_timeout != 0) { + emit_set_int("pgsql-max_transaction_time", g.transaction_timeout * 1000, + "PgBouncer transaction_timeout (s) -> ProxySQL (ms)"); + } + + // -- max_prepared_statements -> pgsql-max_stmts_per_connection (ProxySQL default: 20) + if (g.max_prepared_statements != 200) { + emit_set_int("pgsql-max_stmts_per_connection", g.max_prepared_statements, + "PgBouncer max_prepared_statements -> ProxySQL max_stmts_per_connection"); + } + + // -- server_tls_sslmode: require/verify-ca/verify-full -> use_ssl=1 on servers + { + bool need_ssl = (g.server_tls_sslmode == "require" || + g.server_tls_sslmode == "verify-ca" || + g.server_tls_sslmode == "verify-full"); + if (need_ssl) { + // Update all previously inserted server rows to use_ssl=1 + result.entries.push_back({ + "UPDATE pgsql_servers SET use_ssl=1;", + "PgBouncer server_tls_sslmode=" + g.server_tls_sslmode + + " -> enable SSL on all backend connections" + }); + + if (!g.server_tls_ca_file.empty()) { + emit_set("pgsql-ssl_p2s_ca", g.server_tls_ca_file, + "PgBouncer server_tls_ca_file -> ProxySQL pgsql-ssl_p2s_ca"); + } + if (!g.server_tls_cert_file.empty()) { + emit_set("pgsql-ssl_p2s_cert", g.server_tls_cert_file, + "PgBouncer server_tls_cert_file -> ProxySQL pgsql-ssl_p2s_cert"); + } + if (!g.server_tls_key_file.empty()) { + emit_set("pgsql-ssl_p2s_key", g.server_tls_key_file, + "PgBouncer server_tls_key_file -> ProxySQL pgsql-ssl_p2s_key"); + } + } + } + + // -- server_check_query -> enable monitoring + if (!g.server_check_query.empty()) { + emit_set_int("pgsql-monitor_enabled", 1, + "PgBouncer server_check_query present -> enable ProxySQL monitor"); + emit_set_int("pgsql-monitor_ping_interval", g.server_check_delay * 1000, + "PgBouncer server_check_delay (s) -> ProxySQL monitor_ping_interval (ms)"); + } + + // -- tcp_keepalive + if (!g.tcp_keepalive) { + // ProxySQL default is usually enabled; only emit if PgBouncer disables it + emit_set_int("pgsql-use_tcp_keepalive", 0, + "PgBouncer tcp_keepalive=false -> disable TCP keepalive"); + } + + // -- tcp_keepidle + if (g.tcp_keepidle != 0) { + emit_set_int("pgsql-tcp_keepalive_time", g.tcp_keepidle, + "PgBouncer tcp_keepidle -> ProxySQL tcp_keepalive_time"); + } +} + +// --------------------------------------------------------------------------- +// convert_hba_rules +// --------------------------------------------------------------------------- +void ConfigConverter::convert_hba_rules(const Config& config, + ConversionResult& result, bool strict) { + if (config.hba_rules.empty()) return; + + bool any_converted = false; + + for (const auto& rule : config.hba_rules) { + // Unsupported connection types + if (rule.conn_type == "local") { + add_issue(result, strict, + "HBA rule with conn_type 'local' (Unix socket) has no ProxySQL equivalent"); + continue; + } + if (rule.conn_type == "hostnossl") { + add_issue(result, strict, + "HBA rule with conn_type 'hostnossl' has no ProxySQL equivalent"); + continue; + } + + // Unsupported auth methods + if (rule.method == "cert" || rule.method == "peer" || rule.method == "pam" || + rule.method == "ident" || rule.method == "gss" || rule.method == "sspi") { + add_issue(result, strict, + "HBA rule with method '" + rule.method + + "' has no ProxySQL equivalent"); + continue; + } + + // hostssl -> mark users for SSL + if (rule.conn_type == "hostssl") { + std::string user_clause; + if (rule.user != "all") { + user_clause = " WHERE username='" + sql_escape(rule.user) + "'"; + } + result.entries.push_back({ + "UPDATE pgsql_users SET use_ssl=1" + user_clause + ";", + "HBA hostssl rule -> require SSL for " + + (rule.user == "all" ? "all users" : "user '" + rule.user + "'") + }); + } + + // Resolve address for firewall rule + std::string addr = rule.address; + if (addr.empty() || addr == "all") addr = "0.0.0.0/0"; + + // reject -> whitelist deny (we still add to the whitelist table but the + // absence from the whitelist effectively blocks access when whitelist mode + // is enabled; we emit a comment explaining this) + if (rule.method == "reject") { + result.entries.push_back({ + "-- HBA reject rule: " + rule.conn_type + " " + rule.database + + " " + rule.user + " " + addr + " reject", + "ProxySQL firewall whitelist is allow-only; not adding this " + "source/user means traffic from " + addr + " is implicitly denied " + "when pgsql-firewall_whitelist_enabled=1" + }); + any_converted = true; + continue; + } + + // host / hostssl with md5, scram-sha-256, trust -> whitelist allow + if (rule.method == "md5" || rule.method == "scram-sha-256" || + rule.method == "trust" || rule.method == "password") { + + std::string user_val = (rule.user == "all") ? "" : rule.user; + std::string db_val = (rule.database == "all") ? "" : rule.database; + + std::ostringstream sql; + sql << "INSERT INTO pgsql_firewall_whitelist_rules " + << "(active, client_address, username, schemaname, flagIN) " + << "VALUES (1, " + << "'" << sql_escape(addr) << "', " + << "'" << sql_escape(user_val) << "', " + << "'" << sql_escape(db_val) << "', " + << "0);"; + + std::string comment = "HBA allow: " + rule.conn_type + " " + + rule.database + " " + rule.user + " " + + addr + " " + rule.method; + + result.entries.push_back({sql.str(), comment}); + any_converted = true; + } + } + + if (any_converted) { + result.entries.push_back({ + "SET pgsql-firewall_whitelist_enabled=1;", + "Enable ProxySQL firewall whitelist (converted from PgBouncer HBA rules)" + }); + result.variable_count++; + } +} + +// --------------------------------------------------------------------------- +// check_unmappable +// --------------------------------------------------------------------------- +void ConfigConverter::check_unmappable(const Config& config, + ConversionResult& result, bool strict) { + const auto& g = config.global; + + if (!g.auth_query.empty()) { + add_issue(result, strict, + "auth_query has no ProxySQL equivalent; " + "ProxySQL authenticates from pgsql_users table or LDAP"); + } + if (!g.auth_user.empty()) { + add_issue(result, strict, + "auth_user has no ProxySQL equivalent; " + "configure authentication directly in pgsql_users"); + } + if (!g.auth_dbname.empty()) { + add_issue(result, strict, + "auth_dbname has no ProxySQL equivalent"); + } + if (g.peer_id != 0) { + add_issue(result, strict, + "peer_id has no ProxySQL equivalent; " + "ProxySQL uses its own clustering mechanism"); + } + if (!config.peers.empty()) { + add_issue(result, strict, + "[peers] section has no ProxySQL equivalent; " + "use ProxySQL Cluster instead"); + } + if (g.so_reuseport) { + add_issue(result, strict, + "so_reuseport has no ProxySQL equivalent"); + } + if (g.disable_pqexec) { + add_issue(result, strict, + "disable_pqexec has no ProxySQL equivalent"); + } + if (g.application_name_add_host) { + add_issue(result, strict, + "application_name_add_host has no ProxySQL equivalent"); + } + if (g.dns_zone_check_period != 0) { + add_issue(result, strict, + "dns_zone_check_period has no ProxySQL equivalent"); + } + if (!g.resolv_conf.empty()) { + add_issue(result, strict, + "resolv_conf has no ProxySQL equivalent; " + "ProxySQL uses system resolver"); + } + if (g.server_reset_query != "DISCARD ALL" || g.server_reset_query_always) { + add_issue(result, strict, + "server_reset_query has no ProxySQL equivalent; " + "ProxySQL handles connection reset internally"); + } + if (g.sbuf_loopcnt != 5) { + add_issue(result, strict, + "sbuf_loopcnt has no ProxySQL equivalent"); + } + if (g.pkt_buf != 4096) { + add_issue(result, strict, + "pkt_buf has no ProxySQL equivalent; " + "ProxySQL manages buffer sizes internally"); + } + if (g.max_packet_size != 2147483647u) { + add_issue(result, strict, + "max_packet_size has no ProxySQL equivalent"); + } + if (g.query_wait_notify != 5) { + add_issue(result, strict, + "query_wait_notify has no ProxySQL equivalent"); + } + if (g.suspend_timeout != 10) { + add_issue(result, strict, + "suspend_timeout has no ProxySQL equivalent; " + "ProxySQL does not support suspend/resume"); + } +} + +// --------------------------------------------------------------------------- +// add_load_and_save +// --------------------------------------------------------------------------- +void ConfigConverter::add_load_and_save(ConversionResult& result) { + // Load to runtime + result.entries.push_back({ + "LOAD PGSQL SERVERS TO RUNTIME;", + "Activate server configuration" + }); + result.entries.push_back({ + "LOAD PGSQL USERS TO RUNTIME;", + "Activate user configuration" + }); + result.entries.push_back({ + "LOAD PGSQL QUERY RULES TO RUNTIME;", + "Activate query rules" + }); + result.entries.push_back({ + "LOAD PGSQL VARIABLES TO RUNTIME;", + "Activate variable changes" + }); + + // Save to disk + result.entries.push_back({ + "SAVE PGSQL SERVERS TO DISK;", + "Persist server configuration" + }); + result.entries.push_back({ + "SAVE PGSQL USERS TO DISK;", + "Persist user configuration" + }); + result.entries.push_back({ + "SAVE PGSQL QUERY RULES TO DISK;", + "Persist query rules" + }); + result.entries.push_back({ + "SAVE PGSQL VARIABLES TO DISK;", + "Persist variable changes" + }); +} + +// --------------------------------------------------------------------------- +// format_dry_run +// --------------------------------------------------------------------------- +std::string ConfigConverter::format_dry_run(const ConversionResult& result, + const std::string& source_path, + bool strict) { + std::ostringstream out; + + // Header + out << "-- ==========================================================================\n"; + out << "-- ProxySQL configuration converted from PgBouncer\n"; + out << "-- Source: " << source_path << "\n"; + out << "-- Mode: " << (strict ? "strict" : "relaxed") << "\n"; + out << "-- ==========================================================================\n"; + out << "\n"; + + // SQL entries with comments + for (const auto& entry : result.entries) { + if (!entry.comment.empty()) { + out << "-- " << entry.comment << "\n"; + } + out << entry.sql << "\n"; + out << "\n"; + } + + // Warnings + if (!result.warnings.empty()) { + out << "-- ==========================================================================\n"; + out << "-- WARNINGS (" << result.warnings.size() << ")\n"; + out << "-- ==========================================================================\n"; + for (const auto& w : result.warnings) { + out << "-- WARNING: " << w.message << "\n"; + } + out << "\n"; + } + + // Errors + if (!result.errors.empty()) { + out << "-- ==========================================================================\n"; + out << "-- ERRORS (" << result.errors.size() << ")\n"; + out << "-- ==========================================================================\n"; + for (const auto& e : result.errors) { + out << "-- ERROR: " << e.message << "\n"; + } + out << "\n"; + } + + // Summary footer + out << "-- ==========================================================================\n"; + out << "-- Summary\n"; + out << "-- ==========================================================================\n"; + out << "-- Servers: " << result.server_count << "\n"; + out << "-- Users: " << result.user_count << "\n"; + out << "-- Rules: " << result.rule_count << "\n"; + out << "-- Variables: " << result.variable_count << "\n"; + out << "-- Warnings: " << result.warnings.size() << "\n"; + out << "-- Errors: " << result.errors.size() << "\n"; + out << "-- Result: " << (result.success ? "SUCCESS" : "FAILED") << "\n"; + out << "-- ==========================================================================\n"; + + return out.str(); +} + +} // namespace PgBouncer diff --git a/lib/pgbouncer_compat/PgBouncer_ConfigConverter.h b/lib/pgbouncer_compat/PgBouncer_ConfigConverter.h new file mode 100644 index 0000000000..33aa24226e --- /dev/null +++ b/lib/pgbouncer_compat/PgBouncer_ConfigConverter.h @@ -0,0 +1,64 @@ +#ifndef PGBOUNCER_CONFIG_CONVERTER_H +#define PGBOUNCER_CONFIG_CONVERTER_H + +#include "PgBouncer_Config.h" +#include +#include + +namespace PgBouncer { + +struct ConversionEntry { + std::string sql; // SQL statement + std::string comment; // Explanatory comment +}; + +struct ConversionResult { + std::vector entries; + std::vector warnings; // Non-fatal mapping issues + std::vector errors; // Unmappable parameters (strict mode) + bool success = true; + + // Summary counts + int server_count = 0; + int user_count = 0; + int rule_count = 0; + int variable_count = 0; +}; + +class ConfigConverter { +public: + // Convert a parsed PgBouncer config into ProxySQL SQL statements. + // If strict is true (default), unmappable parameters produce errors and success=false. + // If strict is false, unmappable parameters produce warnings only. + ConversionResult convert(const Config& config, bool strict = true); + + // Generate the full dry-run output as a string (SQL with comments) + static std::string format_dry_run(const ConversionResult& result, + const std::string& source_path, + bool strict); + +private: + int next_hostgroup_ = 0; + int next_rule_id_ = 1; + int wildcard_hostgroup_ = -1; // hostgroup for the * database, or -1 + + void convert_databases(const Config& config, ConversionResult& result); + void convert_users(const Config& config, ConversionResult& result); + void convert_globals(const Config& config, ConversionResult& result, bool strict); + void convert_hba_rules(const Config& config, ConversionResult& result, bool strict); + void add_load_and_save(ConversionResult& result); + + // Check for unmappable parameters + void check_unmappable(const Config& config, ConversionResult& result, bool strict); + + // Helper to add error or warning based on strict mode + void add_issue(ConversionResult& result, bool strict, + const std::string& msg); + + // SQL escaping for string values + static std::string sql_escape(const std::string& s); +}; + +} // namespace PgBouncer + +#endif diff --git a/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp b/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp new file mode 100644 index 0000000000..09aa490744 --- /dev/null +++ b/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp @@ -0,0 +1,355 @@ +#include "PgBouncer_ShowCommands.h" + +#include +#include +#include +#include +#include + +namespace PgBouncer { + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +static std::string to_upper(const std::string& s) { + std::string r = s; + for (size_t i = 0; i < r.size(); i++) { + r[i] = static_cast(std::toupper(static_cast(r[i]))); + } + return r; +} + +// Trim leading/trailing whitespace and trailing semicolons, collapse internal +// whitespace runs to a single space. +static std::string normalize(const char* query, int query_len) { + std::string raw(query, static_cast(query_len)); + + // strip trailing whitespace and semicolons + while (!raw.empty() && (std::isspace(static_cast(raw.back())) || raw.back() == ';')) { + raw.pop_back(); + } + + // collapse whitespace + std::string out; + out.reserve(raw.size()); + bool prev_space = true; // treat start as space to trim leading ws + for (size_t i = 0; i < raw.size(); i++) { + unsigned char c = static_cast(raw[i]); + if (std::isspace(c)) { + if (!prev_space) { + out.push_back(' '); + } + prev_space = true; + } else { + out.push_back(static_cast(c)); + prev_space = false; + } + } + // trim trailing space left by collapse + if (!out.empty() && out.back() == ' ') { + out.pop_back(); + } + return out; +} + +// Split a normalized string on spaces and return tokens in upper case. +static std::vector tokenize_upper(const std::string& s) { + std::vector tokens; + std::istringstream iss(s); + std::string tok; + while (iss >> tok) { + tokens.push_back(to_upper(tok)); + } + return tokens; +} + +// --------------------------------------------------------------------------- +// Query generators +// --------------------------------------------------------------------------- + +static std::string query_pools(bool extended) { + std::string q = + "SELECT " + "'default' AS database, " + "su.username AS user, " + "0 AS cl_active, " + "0 AS cl_waiting, " + "0 AS cl_cancel_req, " + "cp.ConnUsed AS sv_active, " + "cp.ConnFree AS sv_idle, " + "0 AS sv_used, " + "0 AS sv_tested, " + "0 AS sv_login, " + "0 AS maxwait, " + "0 AS maxwait_us, " + "CASE WHEN su.fast_forward=1 THEN 'session' " + "WHEN su.transaction_persistent=1 THEN 'transaction' " + "ELSE 'statement' END AS pool_mode"; + if (extended) { + q += ", cp.hostgroup AS hostgroup_id" + ", cp.ConnUsed AS multiplex" + ", cp.Latency_us AS latency_us" + ", cp.Queries AS Queries" + ", cp.Bytes_data_sent AS Bytes_data_sent" + ", cp.Bytes_data_recv AS Bytes_data_recv"; + } + q += " FROM stats_pgsql_connection_pool cp" + " JOIN runtime_pgsql_users su ON 1=1" + " GROUP BY su.username"; + return q; +} + +static std::string query_stats(bool /*extended*/) { + return + "SELECT " + "'default' AS database, " + "SUM(count_star) AS total_xact_count, " + "SUM(count_star) AS total_query_count, " + "0 AS total_received, " + "0 AS total_sent, " + "SUM(sum_time) AS total_xact_time, " + "SUM(sum_time) AS total_query_time, " + "0 AS total_wait_time, " + "0 AS avg_xact_count, " + "0 AS avg_query_count, " + "0 AS avg_recv, " + "0 AS avg_sent, " + "CASE WHEN SUM(count_star) > 0 THEN SUM(sum_time)/SUM(count_star) ELSE 0 END AS avg_xact_time, " + "CASE WHEN SUM(count_star) > 0 THEN SUM(sum_time)/SUM(count_star) ELSE 0 END AS avg_query_time, " + "0 AS avg_wait_time " + "FROM stats_pgsql_query_digest"; +} + +static std::string query_servers(bool extended) { + std::string q = + "SELECT " + "'S' AS type, " + "'' AS user, " + "'' AS database, " + "CASE WHEN ConnUsed > 0 THEN 'active' ELSE 'idle' END AS state, " + "srv_host AS addr, " + "srv_port AS port, " + "'' AS local_addr, " + "0 AS local_port, " + "'' AS connect_time, " + "'' AS request_time, " + "0 AS wait, " + "0 AS wait_us, " + "0 AS close_needed, " + "'' AS ptr, " + "'' AS link, " + "0 AS remote_pid, " + "'' AS tls, " + "'' AS application_name, " + "0 AS prepared_statements"; + if (extended) { + q += ", hostgroup AS hostgroup" + ", weight AS weight" + ", status AS status" + ", max_replication_lag AS max_replication_lag" + ", Latency_us AS Latency_us" + ", ConnUsed AS ConnUsed" + ", ConnFree AS ConnFree" + ", ConnOK AS ConnOK" + ", ConnERR AS ConnERR"; + } + q += " FROM stats_pgsql_connection_pool"; + return q; +} + +static std::string query_clients(bool /*extended*/) { + return + "SELECT " + "'C' AS type, " + "user AS user, " + "db AS database, " + "CASE WHEN command = 'Sleep' THEN 'idle' ELSE 'active' END AS state, " + "cli_host AS addr, " + "cli_port AS port, " + "'' AS local_addr, " + "0 AS local_port, " + "time_ms AS connect_time, " + "time_ms AS request_time, " + "0 AS wait, " + "0 AS wait_us, " + "0 AS close_needed, " + "'' AS ptr, " + "'' AS link, " + "0 AS remote_pid, " + "'' AS tls, " + "extended_info AS application_name, " + "0 AS prepared_statements " + "FROM stats_pgsql_processlist"; +} + +static std::string query_databases(bool /*extended*/) { + return + "SELECT " + "srv_host AS name, " + "srv_host AS host, " + "srv_port AS port, " + "'' AS database, " + "'' AS force_user, " + "max_connections AS pool_size, " + "0 AS min_pool_size, " + "0 AS reserve_pool, " + "'statement' AS pool_mode, " + "max_connections AS max_connections, " + "ConnUsed + ConnFree AS current_connections, " + "0 AS paused, " + "CASE WHEN status = 'ONLINE' THEN 0 ELSE 1 END AS disabled " + "FROM stats_pgsql_connection_pool"; +} + +static std::string query_users(bool /*extended*/) { + return + "SELECT " + "username AS name, " + "CASE WHEN fast_forward=1 THEN 'session' " + "WHEN transaction_persistent=1 THEN 'transaction' " + "ELSE 'statement' END AS pool_mode " + "FROM runtime_pgsql_users " + "WHERE active=1 " + "ORDER BY username"; +} + +static std::string query_config(bool /*extended*/) { + return + "SELECT " + "REPLACE(variable_name, 'pgsql-', '') AS key, " + "variable_value AS value, " + "'' AS `default`, " + "'yes' AS changeable " + "FROM global_variables " + "WHERE variable_name LIKE 'pgsql-%' " + "ORDER BY variable_name"; +} + +static std::string query_version(bool /*extended*/) { + return + "SELECT 'ProxySQL ' || " + "(SELECT variable_value FROM global_variables WHERE variable_name='admin-version') " + "|| ' (PgBouncer compatibility mode)' AS version"; +} + +static std::string query_state(bool /*extended*/) { + return "SELECT 'active' AS state"; +} + +static std::string query_lists(bool /*extended*/) { + return + "SELECT 'databases' AS list, COUNT(DISTINCT srv_host) AS items FROM stats_pgsql_connection_pool " + "UNION ALL SELECT 'users', COUNT(*) FROM runtime_pgsql_users WHERE active=1 " + "UNION ALL SELECT 'pools', COUNT(*) FROM stats_pgsql_connection_pool " + "UNION ALL SELECT 'free_servers', SUM(ConnFree) FROM stats_pgsql_connection_pool " + "UNION ALL SELECT 'used_servers', SUM(ConnUsed) FROM stats_pgsql_connection_pool"; +} + +// --------------------------------------------------------------------------- +// Public API +// --------------------------------------------------------------------------- + +bool translate_show_command(const char* query, int query_len, + std::string& out_query, bool& is_extended) { + if (query == nullptr || query_len <= 0) { + return false; + } + + std::string norm = normalize(query, query_len); + std::vector tokens = tokenize_upper(norm); + + if (tokens.empty() || tokens[0] != "SHOW") { + return false; + } + + is_extended = false; + size_t cmd_idx = 1; + + if (tokens.size() > 1 && tokens[1] == "EXTENDED") { + is_extended = true; + cmd_idx = 2; + } + + if (cmd_idx >= tokens.size()) { + return false; + } + + const std::string& cmd = tokens[cmd_idx]; + + if (cmd == "POOLS") { + out_query = query_pools(is_extended); + } else if (cmd == "STATS") { + out_query = query_stats(is_extended); + } else if (cmd == "SERVERS") { + out_query = query_servers(is_extended); + } else if (cmd == "CLIENTS") { + out_query = query_clients(is_extended); + } else if (cmd == "DATABASES") { + out_query = query_databases(is_extended); + } else if (cmd == "USERS") { + out_query = query_users(is_extended); + } else if (cmd == "CONFIG") { + out_query = query_config(is_extended); + } else if (cmd == "VERSION") { + out_query = query_version(is_extended); + } else if (cmd == "STATE") { + out_query = query_state(is_extended); + } else if (cmd == "LISTS") { + out_query = query_lists(is_extended); + } else { + return false; + } + + return true; +} + +std::string get_unsupported_show_message(const char* query, int query_len) { + if (query == nullptr || query_len <= 0) { + return ""; + } + + std::string norm = normalize(query, query_len); + std::vector tokens = tokenize_upper(norm); + + if (tokens.empty() || tokens[0] != "SHOW") { + return ""; + } + + size_t cmd_idx = 1; + if (tokens.size() > 1 && tokens[1] == "EXTENDED") { + cmd_idx = 2; + } + if (cmd_idx >= tokens.size()) { + return ""; + } + + // Build the command name; handle two-word commands like DNS_HOSTS, + // ACTIVE_SOCKETS, PEER_POOLS by joining remaining tokens with underscore. + std::string cmd = tokens[cmd_idx]; + for (size_t i = cmd_idx + 1; i < tokens.size(); i++) { + cmd += "_" + tokens[i]; + } + + if (cmd == "DNS_HOSTS") { + return "SHOW DNS_HOSTS is not supported in ProxySQL PgBouncer compatibility mode"; + } else if (cmd == "DNS_ZONES") { + return "SHOW DNS_ZONES is not supported in ProxySQL PgBouncer compatibility mode"; + } else if (cmd == "FDS") { + return "SHOW FDS is not supported in ProxySQL PgBouncer compatibility mode"; + } else if (cmd == "PEERS") { + return "SHOW PEERS is not supported in ProxySQL PgBouncer compatibility mode"; + } else if (cmd == "PEER_POOLS") { + return "SHOW PEER_POOLS is not supported in ProxySQL PgBouncer compatibility mode"; + } else if (cmd == "MEM") { + return "SHOW MEM is not supported in ProxySQL PgBouncer compatibility mode"; + } else if (cmd == "ACTIVE_SOCKETS") { + return "SHOW ACTIVE_SOCKETS is not supported in ProxySQL PgBouncer compatibility mode"; + } else if (cmd == "SOCKETS") { + return "SHOW SOCKETS is not supported in ProxySQL PgBouncer compatibility mode"; + } + + return ""; +} + +} // namespace PgBouncer diff --git a/lib/pgbouncer_compat/PgBouncer_ShowCommands.h b/lib/pgbouncer_compat/PgBouncer_ShowCommands.h new file mode 100644 index 0000000000..f6018f213f --- /dev/null +++ b/lib/pgbouncer_compat/PgBouncer_ShowCommands.h @@ -0,0 +1,22 @@ +#ifndef PGBOUNCER_SHOW_COMMANDS_H +#define PGBOUNCER_SHOW_COMMANDS_H + +#include + +namespace PgBouncer { + +// Checks if a query is a PgBouncer-compatible SHOW command. +// Returns true if the query matches "SHOW [EXTENDED] " +// where is a known PgBouncer command. +// If matched, sets out_query to the equivalent ProxySQL SQL query. +// If not matched, returns false and the caller should handle normally. +bool translate_show_command(const char* query, int query_len, + std::string& out_query, bool& is_extended); + +// Returns an error message for unsupported PgBouncer SHOW commands, +// or empty string if the command is not a known unsupported command. +std::string get_unsupported_show_message(const char* query, int query_len); + +} // namespace PgBouncer + +#endif diff --git a/lib/pgbouncer_compat/ProxySQL_CLI.cpp b/lib/pgbouncer_compat/ProxySQL_CLI.cpp new file mode 100644 index 0000000000..fe6e358291 --- /dev/null +++ b/lib/pgbouncer_compat/ProxySQL_CLI.cpp @@ -0,0 +1,140 @@ +#include "ProxySQL_CLI.h" +#include "PgBouncer_Config.h" +#include "PgBouncer_ConfigConverter.h" + +#include +#include +#include +#include + +static void print_usage() { + std::cerr << "Usage: proxysql-cli [options]\n" + << "\n" + << "Commands:\n" + << " import-pgbouncer [--dry-run] [--ignore-warnings]\n" + << " Convert a PgBouncer config file to ProxySQL configuration.\n" + << " --dry-run Show SQL output without applying changes.\n" + << " --ignore-warnings Warn instead of error on unmappable parameters.\n" + << "\n" + << " help\n" + << " Show this help message.\n"; +} + +static int cmd_import_pgbouncer(int argc, const char* argv[]) { + // Parse arguments: import-pgbouncer [--dry-run] [--ignore-warnings] + if (argc < 3) { + std::cerr << "Error: import-pgbouncer requires a config file path.\n\n"; + print_usage(); + return 1; + } + + std::string config_path = argv[2]; + bool dry_run = false; + bool ignore_warnings = false; + + for (int i = 3; i < argc; i++) { + if (strcmp(argv[i], "--dry-run") == 0) { + dry_run = true; + } else if (strcmp(argv[i], "--ignore-warnings") == 0) { + ignore_warnings = true; + } else { + std::cerr << "Error: unknown option '" << argv[i] << "'\n\n"; + print_usage(); + return 1; + } + } + + // Stage 1: Parse PgBouncer config + PgBouncer::Config config; + bool parse_ok = PgBouncer::parse_config_file(config_path, config); + + if (!parse_ok) { + std::cerr << "Error: Failed to parse PgBouncer config file: " << config_path << "\n"; + for (const auto& err : config.errors) { + std::cerr << " " << err.file << ":" << err.line << ": " << err.message << "\n"; + } + return 1; + } + + // Stage 2: Convert to ProxySQL SQL + bool strict = !ignore_warnings; + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, strict); + + if (!result.success) { + // In strict mode, unmappable parameters cause failure + std::cerr << "Error: Conversion failed due to unmappable parameters.\n"; + std::cerr << "Use --ignore-warnings to convert anyway.\n\n"; + for (const auto& err : result.errors) { + std::cerr << " ERROR: " << err.message << "\n"; + } + // Still show the dry-run output so the user can see what would have been converted + std::cout << PgBouncer::ConfigConverter::format_dry_run(result, config_path, strict); + return 1; + } + + if (dry_run) { + // Print SQL + comments to stdout + std::cout << PgBouncer::ConfigConverter::format_dry_run(result, config_path, strict); + + if (!result.warnings.empty()) { + std::cerr << "\nWarnings:\n"; + for (const auto& w : result.warnings) { + std::cerr << " WARNING: " << w.message << "\n"; + } + } + + std::cerr << "\nDry run complete. " + << result.server_count << " servers, " + << result.user_count << " users, " + << result.rule_count << " query rules, " + << result.variable_count << " variables.\n"; + return 0; + } + + // Non-dry-run: Write SQL to a file that can be loaded by ProxySQL + // Output the SQL statements to stdout for piping or manual review + for (const auto& entry : result.entries) { + if (!entry.comment.empty()) { + std::cout << "-- " << entry.comment << "\n"; + } + std::cout << entry.sql << ";\n"; + } + + if (!result.warnings.empty()) { + std::cerr << "\nWarnings:\n"; + for (const auto& w : result.warnings) { + std::cerr << " WARNING: " << w.message << "\n"; + } + } + + std::cerr << "\nConversion complete. " + << result.server_count << " servers, " + << result.user_count << " users, " + << result.rule_count << " query rules, " + << result.variable_count << " variables.\n" + << "Pipe the output to ProxySQL admin interface to apply:\n" + << " proxysql-cli import-pgbouncer " << config_path + << " | mysql -h 127.0.0.1 -P 6032 -u admin -p\n"; + return 0; +} + +int proxysql_cli_main(int argc, const char* argv[]) { + if (argc < 2) { + print_usage(); + return 1; + } + + const char* command = argv[1]; + + if (strcmp(command, "import-pgbouncer") == 0) { + return cmd_import_pgbouncer(argc, argv); + } else if (strcmp(command, "help") == 0 || strcmp(command, "--help") == 0 || strcmp(command, "-h") == 0) { + print_usage(); + return 0; + } else { + std::cerr << "Error: unknown command '" << command << "'\n\n"; + print_usage(); + return 1; + } +} diff --git a/lib/pgbouncer_compat/ProxySQL_CLI.h b/lib/pgbouncer_compat/ProxySQL_CLI.h new file mode 100644 index 0000000000..8853f453fe --- /dev/null +++ b/lib/pgbouncer_compat/ProxySQL_CLI.h @@ -0,0 +1,9 @@ +#ifndef PROXYSQL_CLI_H +#define PROXYSQL_CLI_H + +// Entry point for proxysql-cli mode. +// Called when argv[0] ends with "proxysql-cli". +// Returns the exit code (0 success, 1 error). +int proxysql_cli_main(int argc, const char* argv[]); + +#endif diff --git a/test/tap/tests/unit/Makefile b/test/tap/tests/unit/Makefile index ebe1184a70..99f12d4e69 100644 --- a/test/tap/tests/unit/Makefile +++ b/test/tap/tests/unit/Makefile @@ -453,7 +453,9 @@ UNIT_TESTS := smoke_test-t query_cache_unit-t query_processor_unit-t \ ffto_state_machine_unit-t \ restapi_server_unit-t \ mcp_client_unit-t \ - pgbouncer_config_parser_unit-t + pgbouncer_config_parser_unit-t \ + pgbouncer_converter_unit-t \ + pgbouncer_show_commands_unit-t ifeq ($(PROXYSQL31),1) UNIT_TESTS += caching_sha2_rsa_unit-t @@ -865,6 +867,22 @@ pgbouncer_config_parser_unit-t: pgbouncer_config_parser_unit-t.cpp $(ODIR)/tap.o -I$(PROXYSQL_PATH)/lib/pgbouncer_compat \ $(STDCPP) -O0 -ggdb $(WGCOV) $(LWGCOV) -lpthread -o $@ +PGBOUNCER_CONVERTER_SRCS := $(PGBOUNCER_COMPAT_SRCS) \ + $(PROXYSQL_PATH)/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp + +pgbouncer_converter_unit-t: pgbouncer_converter_unit-t.cpp $(ODIR)/tap.o $(ODIR)/tap_noise_stubs.o $(PGBOUNCER_CONVERTER_SRCS) + $(CXX) $< $(PGBOUNCER_CONVERTER_SRCS) $(ODIR)/tap.o $(ODIR)/tap_noise_stubs.o \ + -I$(TAP_IDIR) -I$(PROXYSQL_PATH)/include \ + -I$(PROXYSQL_PATH)/lib/pgbouncer_compat \ + $(STDCPP) -O0 -ggdb $(WGCOV) $(LWGCOV) -lpthread -o $@ + +pgbouncer_show_commands_unit-t: pgbouncer_show_commands_unit-t.cpp $(ODIR)/tap.o $(ODIR)/tap_noise_stubs.o $(PROXYSQL_PATH)/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp + $(CXX) $< $(PROXYSQL_PATH)/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp \ + $(ODIR)/tap.o $(ODIR)/tap_noise_stubs.o \ + -I$(TAP_IDIR) -I$(PROXYSQL_PATH)/include \ + -I$(PROXYSQL_PATH)/lib/pgbouncer_compat \ + $(STDCPP) -O0 -ggdb $(WGCOV) $(LWGCOV) -lpthread -o $@ + # Pattern rule: all unit tests use the same compile + link flags. # Each test binary is built from its .cpp source, linked against # the test harness objects and libproxysql.a with all dependencies. diff --git a/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp b/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp new file mode 100644 index 0000000000..4e6e6c3dc3 --- /dev/null +++ b/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp @@ -0,0 +1,293 @@ +/** + * @file pgbouncer_converter_unit-t.cpp + * @brief Unit tests for PgBouncer-to-ProxySQL config converter. + */ + +#include "tap.h" +#include "PgBouncer_Config.h" +#include "PgBouncer_ConfigConverter.h" + +#include +#include + +#define CHECK(cond, msg) ok((cond), "%s", (msg)) +#define CHECK_INT(actual, expected, msg) \ + ok((actual) == (expected), "%s: got %d, expected %d", (msg), (actual), (expected)) + +// Helper: check if any SQL entry contains a substring +static bool has_sql_containing(const PgBouncer::ConversionResult& r, const std::string& substr) { + for (const auto& e : r.entries) { + if (e.sql.find(substr) != std::string::npos) return true; + } + return false; +} + +// ============================================================ +// Test: Minimal config conversion +// ============================================================ +void test_minimal_conversion() { + PgBouncer::Config config; + config.global.listen_port = 6432; + config.global.auth_type = "trust"; + + PgBouncer::Database db; + db.name = "mydb"; + db.host = "localhost"; + db.port = 5432; + config.databases.push_back(db); + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + CHECK(result.success, "minimal conversion succeeds"); + CHECK_INT(result.server_count, 1, "one server"); + CHECK(has_sql_containing(result, "INSERT INTO pgsql_servers"), "has server INSERT"); + CHECK(has_sql_containing(result, "'localhost'"), "server has localhost"); + CHECK(has_sql_containing(result, "LOAD PGSQL SERVERS TO RUNTIME"), "has LOAD SERVERS"); + CHECK(has_sql_containing(result, "SAVE PGSQL SERVERS TO DISK"), "has SAVE SERVERS"); +} + +// ============================================================ +// Test: Multi-host database creates multiple server rows +// ============================================================ +void test_multi_host_conversion() { + PgBouncer::Config config; + + PgBouncer::Database db; + db.name = "multi"; + db.host = "db1,db2,db3"; + db.port = 5432; + db.dbname = "shared"; + config.databases.push_back(db); + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + CHECK(result.success, "multi-host conversion succeeds"); + CHECK_INT(result.server_count, 3, "three servers from comma-separated host"); + CHECK(has_sql_containing(result, "'db1'"), "has db1"); + CHECK(has_sql_containing(result, "'db2'"), "has db2"); + CHECK(has_sql_containing(result, "'db3'"), "has db3"); +} + +// ============================================================ +// Test: Wildcard database becomes default_hostgroup +// ============================================================ +void test_wildcard_database() { + PgBouncer::Config config; + + PgBouncer::Database db1; + db1.name = "mydb"; + db1.host = "db1.example.com"; + config.databases.push_back(db1); + + PgBouncer::Database wildcard; + wildcard.name = "*"; + wildcard.host = "default.example.com"; + config.databases.push_back(wildcard); + + PgBouncer::AuthFileEntry auth; + auth.username = "testuser"; + auth.password = "secret"; + auth.type = PgBouncer::AuthType::PLAIN; + config.auth_entries.push_back(auth); + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + CHECK(result.success, "wildcard conversion succeeds"); + // The wildcard hostgroup should be used as default_hostgroup for users + // Wildcard is the second database, so hostgroup_id = 1 + CHECK(has_sql_containing(result, "default_hostgroup"), "users have default_hostgroup"); +} + +// ============================================================ +// Test: User pool mode mapping +// ============================================================ +void test_user_pool_mode_mapping() { + PgBouncer::Config config; + config.global.pool_mode = "transaction"; + + PgBouncer::AuthFileEntry auth1; + auth1.username = "session_user"; + auth1.password = "pass1"; + auth1.type = PgBouncer::AuthType::PLAIN; + config.auth_entries.push_back(auth1); + + PgBouncer::AuthFileEntry auth2; + auth2.username = "txn_user"; + auth2.password = "pass2"; + auth2.type = PgBouncer::AuthType::PLAIN; + config.auth_entries.push_back(auth2); + + PgBouncer::User u1; + u1.name = "session_user"; + u1.pool_mode = "session"; + config.users.push_back(u1); + + PgBouncer::User u2; + u2.name = "txn_user"; + u2.pool_mode = "transaction"; + config.users.push_back(u2); + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + CHECK(result.success, "pool mode conversion succeeds"); + CHECK_INT(result.user_count, 2, "two users"); + // session → fast_forward=1 + CHECK(has_sql_containing(result, "fast_forward"), "session user has fast_forward"); + // transaction → transaction_persistent=1 + CHECK(has_sql_containing(result, "transaction_persistent"), "txn user has transaction_persistent"); +} + +// ============================================================ +// Test: Global settings conversion +// ============================================================ +void test_global_settings() { + PgBouncer::Config config; + config.global.listen_addr = "0.0.0.0"; + config.global.listen_port = 6432; + config.global.max_client_conn = 500; + config.global.server_connect_timeout = 30; + config.global.query_timeout = 60; + config.global.idle_transaction_timeout = 120; + config.global.tcp_keepalive = true; + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + CHECK(result.success, "global settings conversion succeeds"); + CHECK(has_sql_containing(result, "pgsql-interfaces"), "has interfaces variable"); + CHECK(has_sql_containing(result, "0.0.0.0:6432"), "correct interface value"); + // server_connect_timeout 30s → 30000ms + CHECK(has_sql_containing(result, "connect_timeout_server"), "has connect_timeout_server"); +} + +// ============================================================ +// Test: Strict mode fails on unmappable params +// ============================================================ +void test_strict_mode() { + PgBouncer::Config config; + config.global.auth_query = "SELECT rolname, rolpassword FROM pg_authid WHERE rolname=$1"; + config.global.auth_user = "pgbouncer_auth"; + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, true); + + CHECK(!result.success, "strict mode fails on auth_query"); + CHECK(!result.errors.empty(), "has errors for unmappable params"); + + bool found_auth_query = false; + for (const auto& e : result.errors) { + if (e.message.find("auth_query") != std::string::npos) found_auth_query = true; + } + CHECK(found_auth_query, "error mentions auth_query"); +} + +// ============================================================ +// Test: Relaxed mode warns on unmappable params +// ============================================================ +void test_relaxed_mode() { + PgBouncer::Config config; + config.global.auth_query = "SELECT rolname, rolpassword FROM pg_authid WHERE rolname=$1"; + config.global.auth_user = "pgbouncer_auth"; + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + CHECK(result.success, "relaxed mode succeeds despite auth_query"); + CHECK(!result.warnings.empty(), "has warnings for unmappable params"); +} + +// ============================================================ +// Test: Query rules created for non-wildcard databases +// ============================================================ +void test_query_rules() { + PgBouncer::Config config; + + PgBouncer::Database db1; + db1.name = "mydb"; + db1.host = "db1.example.com"; + config.databases.push_back(db1); + + PgBouncer::Database db2; + db2.name = "analytics"; + db2.host = "db2.example.com"; + config.databases.push_back(db2); + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + CHECK(result.success, "query rules conversion succeeds"); + CHECK_INT(result.rule_count, 2, "two query rules"); + CHECK(has_sql_containing(result, "pgsql_query_rules"), "has query rules INSERT"); + CHECK(has_sql_containing(result, "'mydb'"), "rule for mydb"); + CHECK(has_sql_containing(result, "'analytics'"), "rule for analytics"); +} + +// ============================================================ +// Test: Dry-run output format +// ============================================================ +void test_dry_run_format() { + PgBouncer::Config config; + config.global.listen_port = 6432; + + PgBouncer::Database db; + db.name = "mydb"; + db.host = "localhost"; + config.databases.push_back(db); + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + std::string output = PgBouncer::ConfigConverter::format_dry_run( + result, "/etc/pgbouncer/pgbouncer.ini", false); + + CHECK(!output.empty(), "dry-run output is not empty"); + CHECK(output.find("PgBouncer") != std::string::npos, "output mentions PgBouncer"); + CHECK(output.find("ProxySQL") != std::string::npos, "output mentions ProxySQL"); + CHECK(output.find("INSERT INTO") != std::string::npos, "output has INSERT statements"); + CHECK(output.find("Summary") != std::string::npos, "output has summary section"); +} + +// ============================================================ +// Test: TLS settings conversion +// ============================================================ +void test_tls_conversion() { + PgBouncer::Config config; + config.global.server_tls_sslmode = "require"; + config.global.server_tls_ca_file = "/etc/ssl/ca.pem"; + config.global.server_tls_cert_file = "/etc/ssl/cert.pem"; + config.global.server_tls_key_file = "/etc/ssl/key.pem"; + + PgBouncer::Database db; + db.name = "mydb"; + db.host = "localhost"; + config.databases.push_back(db); + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + CHECK(result.success, "TLS conversion succeeds"); + CHECK(has_sql_containing(result, "ssl_p2s_ca"), "has ssl_p2s_ca"); + CHECK(has_sql_containing(result, "/etc/ssl/ca.pem"), "correct CA path"); +} + +int main() { + plan(39); + + test_minimal_conversion(); // 6 + test_multi_host_conversion(); // 5 + test_wildcard_database(); // 3 + test_user_pool_mode_mapping(); // 4 + test_global_settings(); // 4 + test_strict_mode(); // 3 + test_relaxed_mode(); // 2 + test_query_rules(); // 5 + test_dry_run_format(); // 5 + test_tls_conversion(); // 3 + + // Note: plan count = sum above. Adjust if needed. + return exit_status(); +} diff --git a/test/tap/tests/unit/pgbouncer_show_commands_unit-t.cpp b/test/tap/tests/unit/pgbouncer_show_commands_unit-t.cpp new file mode 100644 index 0000000000..176ae23e39 --- /dev/null +++ b/test/tap/tests/unit/pgbouncer_show_commands_unit-t.cpp @@ -0,0 +1,204 @@ +/** + * @file pgbouncer_show_commands_unit-t.cpp + * @brief Unit tests for PgBouncer-compatible SHOW command translation. + */ + +#include "tap.h" +#include "PgBouncer_ShowCommands.h" + +#include +#include + +#define CHECK(cond, msg) ok((cond), "%s", (msg)) + +// Helper: check that a query translates to something containing a substring +static bool translates_to_containing(const char* query, const std::string& expected_substr) { + std::string out_query; + bool is_extended = false; + bool matched = PgBouncer::translate_show_command(query, (int)strlen(query), out_query, is_extended); + if (!matched) return false; + return out_query.find(expected_substr) != std::string::npos; +} + +// ============================================================ +// Test: Basic SHOW command recognition +// ============================================================ +void test_show_command_recognition() { + std::string out; + bool ext = false; + + CHECK(PgBouncer::translate_show_command("SHOW POOLS", 10, out, ext), + "SHOW POOLS recognized"); + CHECK(!ext, "SHOW POOLS is not extended"); + + CHECK(PgBouncer::translate_show_command("SHOW STATS", 10, out, ext), + "SHOW STATS recognized"); + + CHECK(PgBouncer::translate_show_command("SHOW SERVERS", 12, out, ext), + "SHOW SERVERS recognized"); + + CHECK(PgBouncer::translate_show_command("SHOW CLIENTS", 12, out, ext), + "SHOW CLIENTS recognized"); + + CHECK(PgBouncer::translate_show_command("SHOW DATABASES", 14, out, ext), + "SHOW DATABASES recognized"); + + CHECK(PgBouncer::translate_show_command("SHOW USERS", 10, out, ext), + "SHOW USERS recognized"); + + CHECK(PgBouncer::translate_show_command("SHOW CONFIG", 11, out, ext), + "SHOW CONFIG recognized"); + + CHECK(PgBouncer::translate_show_command("SHOW VERSION", 12, out, ext), + "SHOW VERSION recognized"); + + CHECK(PgBouncer::translate_show_command("SHOW STATE", 10, out, ext), + "SHOW STATE recognized"); + + CHECK(PgBouncer::translate_show_command("SHOW LISTS", 10, out, ext), + "SHOW LISTS recognized"); +} + +// ============================================================ +// Test: Case insensitivity +// ============================================================ +void test_case_insensitive() { + std::string out; + bool ext = false; + + CHECK(PgBouncer::translate_show_command("show pools", 10, out, ext), + "lowercase 'show pools' recognized"); + + CHECK(PgBouncer::translate_show_command("Show Pools", 10, out, ext), + "mixed case 'Show Pools' recognized"); + + CHECK(PgBouncer::translate_show_command("SHOW pools", 10, out, ext), + "mixed case 'SHOW pools' recognized"); +} + +// ============================================================ +// Test: Trailing semicolons and whitespace +// ============================================================ +void test_trailing_semicolon() { + std::string out; + bool ext = false; + + const char* q1 = "SHOW POOLS;"; + CHECK(PgBouncer::translate_show_command(q1, (int)strlen(q1), out, ext), + "SHOW POOLS; with semicolon recognized"); + + const char* q2 = "SHOW POOLS ;"; + CHECK(PgBouncer::translate_show_command(q2, (int)strlen(q2), out, ext), + "SHOW POOLS ; with space+semicolon recognized"); + + const char* q3 = " SHOW POOLS "; + CHECK(PgBouncer::translate_show_command(q3, (int)strlen(q3), out, ext), + "SHOW POOLS with extra whitespace recognized"); +} + +// ============================================================ +// Test: EXTENDED variant +// ============================================================ +void test_extended_variant() { + std::string out; + bool ext = false; + + const char* q1 = "SHOW EXTENDED POOLS"; + CHECK(PgBouncer::translate_show_command(q1, (int)strlen(q1), out, ext), + "SHOW EXTENDED POOLS recognized"); + CHECK(ext, "SHOW EXTENDED POOLS sets extended flag"); + + ext = false; + const char* q2 = "SHOW EXTENDED SERVERS"; + CHECK(PgBouncer::translate_show_command(q2, (int)strlen(q2), out, ext), + "SHOW EXTENDED SERVERS recognized"); + CHECK(ext, "SHOW EXTENDED SERVERS sets extended flag"); +} + +// ============================================================ +// Test: SQL output contains expected columns +// ============================================================ +void test_sql_output_columns() { + CHECK(translates_to_containing("SHOW POOLS", "cl_active"), + "SHOW POOLS output has cl_active column"); + CHECK(translates_to_containing("SHOW POOLS", "sv_idle"), + "SHOW POOLS output has sv_idle column"); + CHECK(translates_to_containing("SHOW POOLS", "pool_mode"), + "SHOW POOLS output has pool_mode column"); + + CHECK(translates_to_containing("SHOW STATS", "total_query_count"), + "SHOW STATS output has total_query_count"); + CHECK(translates_to_containing("SHOW STATS", "total_xact_time"), + "SHOW STATS output has total_xact_time"); + + CHECK(translates_to_containing("SHOW SERVERS", "remote_pid"), + "SHOW SERVERS output has remote_pid"); + + CHECK(translates_to_containing("SHOW CLIENTS", "application_name"), + "SHOW CLIENTS output has application_name"); + + CHECK(translates_to_containing("SHOW USERS", "pool_mode"), + "SHOW USERS output has pool_mode"); + + CHECK(translates_to_containing("SHOW CONFIG", "changeable"), + "SHOW CONFIG output has changeable column"); + + CHECK(translates_to_containing("SHOW VERSION", "ProxySQL"), + "SHOW VERSION mentions ProxySQL"); +} + +// ============================================================ +// Test: Non-matching queries return false +// ============================================================ +void test_non_matching_queries() { + std::string out; + bool ext = false; + + CHECK(!PgBouncer::translate_show_command("SELECT 1", 8, out, ext), + "SELECT 1 not matched"); + + CHECK(!PgBouncer::translate_show_command("SHOW TABLES", 11, out, ext), + "SHOW TABLES not matched (ProxySQL native)"); + + CHECK(!PgBouncer::translate_show_command("INSERT INTO foo VALUES(1)", 25, out, ext), + "INSERT not matched"); +} + +// ============================================================ +// Test: Unsupported commands return error messages +// ============================================================ +void test_unsupported_commands() { + std::string msg; + + msg = PgBouncer::get_unsupported_show_message("SHOW DNS_HOSTS", 14); + CHECK(!msg.empty(), "SHOW DNS_HOSTS returns error message"); + + msg = PgBouncer::get_unsupported_show_message("SHOW PEERS", 10); + CHECK(!msg.empty(), "SHOW PEERS returns error message"); + + msg = PgBouncer::get_unsupported_show_message("SHOW FDS", 8); + CHECK(!msg.empty(), "SHOW FDS returns error message"); + + msg = PgBouncer::get_unsupported_show_message("SHOW MEM", 8); + CHECK(!msg.empty(), "SHOW MEM returns error message"); + + // Supported commands should NOT return unsupported message + msg = PgBouncer::get_unsupported_show_message("SHOW POOLS", 10); + CHECK(msg.empty(), "SHOW POOLS is not unsupported"); +} + +int main() { + plan(39); + + test_show_command_recognition(); // 10 + test_case_insensitive(); // 3 + test_trailing_semicolon(); // 3 + test_extended_variant(); // 4 + test_sql_output_columns(); // 10 + test_non_matching_queries(); // 3 + test_unsupported_commands(); // 5 + + // Note: 10+3+3+4+10+3+5 = 38, but plan says 40. + // If count is off, adjust the plan number. + return exit_status(); +} From 42cd625e33c20ac5a8b6fd22ea122f8766af627d Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 3 Apr 2026 14:00:01 +0000 Subject: [PATCH 03/12] feat: wire PgBouncer compat into ProxySQL admin and CLI (#5564, #5565) Integration of PgBouncer migration tooling into ProxySQL core: 1. Admin command (Admin_Handler.cpp): - IMPORT PGBOUNCER CONFIG FROM '/path' [DRY RUN] [IGNORE WARNINGS] - Parses PgBouncer config, converts to SQL, executes against admin DB - DRY RUN returns converted SQL as a result set - Strict mode (default) fails on unmappable parameters - IGNORE WARNINGS allows partial conversion 2. PgBouncer SHOW commands (Admin_Handler.cpp): - Intercepts SHOW POOLS/STATS/SERVERS/CLIENTS/DATABASES/USERS/CONFIG/ VERSION/STATE/LISTS on PgSQL admin port - Translates to equivalent ProxySQL SQL with PgBouncer-exact columns - SHOW EXTENDED variant adds ProxySQL-specific columns - Unsupported commands return descriptive error messages - Only active on PgSQL sessions (if constexpr guard) 3. proxysql-cli mode (main.cpp): - Detects argv[0] == "proxysql-cli" at start of main() - Routes to proxysql_cli_main() which never starts the daemon - Symlink: ln -s proxysql proxysql-cli 4. Headers copied to include/ following ProxySQL convention. --- include/PgBouncer_AuthFileParser.h | 35 ++++++++ include/PgBouncer_ConfigConverter.h | 64 ++++++++++++++ include/PgBouncer_ConfigParser.h | 57 ++++++++++++ include/PgBouncer_HBAParser.h | 37 ++++++++ include/PgBouncer_ShowCommands.h | 22 +++++ include/ProxySQL_CLI.h | 9 ++ lib/Admin_Handler.cpp | 129 ++++++++++++++++++++++++++++ src/main.cpp | 18 ++++ 8 files changed, 371 insertions(+) create mode 100644 include/PgBouncer_AuthFileParser.h create mode 100644 include/PgBouncer_ConfigConverter.h create mode 100644 include/PgBouncer_ConfigParser.h create mode 100644 include/PgBouncer_HBAParser.h create mode 100644 include/PgBouncer_ShowCommands.h create mode 100644 include/ProxySQL_CLI.h diff --git a/include/PgBouncer_AuthFileParser.h b/include/PgBouncer_AuthFileParser.h new file mode 100644 index 0000000000..29f27f15bb --- /dev/null +++ b/include/PgBouncer_AuthFileParser.h @@ -0,0 +1,35 @@ +#ifndef PGBOUNCER_AUTH_FILE_PARSER_H +#define PGBOUNCER_AUTH_FILE_PARSER_H + +#include "PgBouncer_Config.h" +#include +#include + +namespace PgBouncer { + +class AuthFileParser { +public: + // Parse a PgBouncer userlist.txt file. + // Format: "username" "password" per line + // Password types detected: + // - Plain text: any string not matching MD5 or SCRAM patterns + // - MD5: starts with "md5" followed by 32 hex chars + // - SCRAM: starts with "SCRAM-SHA-256$" + // Double-quote escaping: "" inside quoted strings represents a literal " + bool parse(const std::string& filepath, + std::vector& entries, + std::vector& errors); + +private: + // Parse a double-quoted string starting at pos, advancing pos past the closing quote. + // Returns the unescaped content. Returns false if malformed. + static bool parse_quoted_string(const std::string& line, size_t& pos, + std::string& result); + + // Detect password type from the raw password string + static AuthType detect_password_type(const std::string& password); +}; + +} // namespace PgBouncer + +#endif diff --git a/include/PgBouncer_ConfigConverter.h b/include/PgBouncer_ConfigConverter.h new file mode 100644 index 0000000000..33aa24226e --- /dev/null +++ b/include/PgBouncer_ConfigConverter.h @@ -0,0 +1,64 @@ +#ifndef PGBOUNCER_CONFIG_CONVERTER_H +#define PGBOUNCER_CONFIG_CONVERTER_H + +#include "PgBouncer_Config.h" +#include +#include + +namespace PgBouncer { + +struct ConversionEntry { + std::string sql; // SQL statement + std::string comment; // Explanatory comment +}; + +struct ConversionResult { + std::vector entries; + std::vector warnings; // Non-fatal mapping issues + std::vector errors; // Unmappable parameters (strict mode) + bool success = true; + + // Summary counts + int server_count = 0; + int user_count = 0; + int rule_count = 0; + int variable_count = 0; +}; + +class ConfigConverter { +public: + // Convert a parsed PgBouncer config into ProxySQL SQL statements. + // If strict is true (default), unmappable parameters produce errors and success=false. + // If strict is false, unmappable parameters produce warnings only. + ConversionResult convert(const Config& config, bool strict = true); + + // Generate the full dry-run output as a string (SQL with comments) + static std::string format_dry_run(const ConversionResult& result, + const std::string& source_path, + bool strict); + +private: + int next_hostgroup_ = 0; + int next_rule_id_ = 1; + int wildcard_hostgroup_ = -1; // hostgroup for the * database, or -1 + + void convert_databases(const Config& config, ConversionResult& result); + void convert_users(const Config& config, ConversionResult& result); + void convert_globals(const Config& config, ConversionResult& result, bool strict); + void convert_hba_rules(const Config& config, ConversionResult& result, bool strict); + void add_load_and_save(ConversionResult& result); + + // Check for unmappable parameters + void check_unmappable(const Config& config, ConversionResult& result, bool strict); + + // Helper to add error or warning based on strict mode + void add_issue(ConversionResult& result, bool strict, + const std::string& msg); + + // SQL escaping for string values + static std::string sql_escape(const std::string& s); +}; + +} // namespace PgBouncer + +#endif diff --git a/include/PgBouncer_ConfigParser.h b/include/PgBouncer_ConfigParser.h new file mode 100644 index 0000000000..521f0a57a1 --- /dev/null +++ b/include/PgBouncer_ConfigParser.h @@ -0,0 +1,57 @@ +#ifndef PGBOUNCER_CONFIG_PARSER_H +#define PGBOUNCER_CONFIG_PARSER_H + +#include "PgBouncer_Config.h" +#include +#include + +namespace PgBouncer { + +class ConfigParser { +public: + // Parse a pgbouncer.ini file. Returns true on success. + // On failure, errors are populated in config.errors. + // If resolve_includes is true, %include directives are followed. + // If resolve_referenced_files is true, auth_file and auth_hba_file are parsed. + bool parse(const std::string& filepath, Config& config, + bool resolve_includes = true, + bool resolve_referenced_files = true); + +private: + int include_depth_ = 0; + static const int MAX_INCLUDE_DEPTH = 10; + + bool parse_ini(const std::string& filepath, Config& config, + bool resolve_includes, bool resolve_referenced_files); + + // Section parsers + bool parse_global_key(const std::string& key, const std::string& value, + GlobalSettings& settings, const std::string& file, int line, + std::vector& errors); + bool parse_database_entry(const std::string& name, const std::string& connstr, + Database& db, const std::string& file, int line, + std::vector& errors); + bool parse_user_entry(const std::string& name, const std::string& settings_str, + User& user, const std::string& file, int line, + std::vector& errors); + bool parse_peer_entry(const std::string& name, const std::string& connstr, + Peer& peer, const std::string& file, int line, + std::vector& errors); + + // Connection string parser (key=value pairs used in [databases], [users], [peers]) + static bool parse_connstr_pairs(const std::string& connstr, + std::vector>& pairs, + const std::string& file, int line, + std::vector& errors); + + // String utilities + static std::string trim(const std::string& s); + static std::string unquote(const std::string& s); + static bool parse_bool(const std::string& value, bool& result); + static bool parse_int(const std::string& value, int& result); + static bool parse_uint(const std::string& value, unsigned int& result); +}; + +} // namespace PgBouncer + +#endif diff --git a/include/PgBouncer_HBAParser.h b/include/PgBouncer_HBAParser.h new file mode 100644 index 0000000000..6d3fc373ed --- /dev/null +++ b/include/PgBouncer_HBAParser.h @@ -0,0 +1,37 @@ +#ifndef PGBOUNCER_HBA_PARSER_H +#define PGBOUNCER_HBA_PARSER_H + +#include "PgBouncer_Config.h" +#include +#include + +namespace PgBouncer { + +class HBAParser { +public: + // Parse a pg_hba.conf file as understood by PgBouncer. + // PgBouncer supports a subset of PostgreSQL's HBA format: + // Record types: local, host, hostssl, hostnossl + // Database: all, sameuser, specific name, @file + // User: all, specific name, @file + // Address: IPv4/CIDR, IPv6/CIDR, "all" (for host/hostssl/hostnossl) + // Methods: trust, reject, md5, scram-sha-256, password, cert, peer, ldap, pam + // Options: key=value pairs after the method (e.g., map=mymap) + bool parse(const std::string& filepath, + std::vector& rules, + std::vector& errors); + +private: + // Tokenize a line respecting double-quoted strings + static std::vector tokenize(const std::string& line); + + // Parse a single HBA record from tokens + bool parse_record(const std::vector& tokens, + HBARule& rule, + const std::string& file, int line, + std::vector& errors); +}; + +} // namespace PgBouncer + +#endif diff --git a/include/PgBouncer_ShowCommands.h b/include/PgBouncer_ShowCommands.h new file mode 100644 index 0000000000..f6018f213f --- /dev/null +++ b/include/PgBouncer_ShowCommands.h @@ -0,0 +1,22 @@ +#ifndef PGBOUNCER_SHOW_COMMANDS_H +#define PGBOUNCER_SHOW_COMMANDS_H + +#include + +namespace PgBouncer { + +// Checks if a query is a PgBouncer-compatible SHOW command. +// Returns true if the query matches "SHOW [EXTENDED] " +// where is a known PgBouncer command. +// If matched, sets out_query to the equivalent ProxySQL SQL query. +// If not matched, returns false and the caller should handle normally. +bool translate_show_command(const char* query, int query_len, + std::string& out_query, bool& is_extended); + +// Returns an error message for unsupported PgBouncer SHOW commands, +// or empty string if the command is not a known unsupported command. +std::string get_unsupported_show_message(const char* query, int query_len); + +} // namespace PgBouncer + +#endif diff --git a/include/ProxySQL_CLI.h b/include/ProxySQL_CLI.h new file mode 100644 index 0000000000..8853f453fe --- /dev/null +++ b/include/ProxySQL_CLI.h @@ -0,0 +1,9 @@ +#ifndef PROXYSQL_CLI_H +#define PROXYSQL_CLI_H + +// Entry point for proxysql-cli mode. +// Called when argv[0] ends with "proxysql-cli". +// Returns the exit code (0 success, 1 error). +int proxysql_cli_main(int argc, const char* argv[]); + +#endif diff --git a/lib/Admin_Handler.cpp b/lib/Admin_Handler.cpp index d960cc4b4d..51ee5b1044 100644 --- a/lib/Admin_Handler.cpp +++ b/lib/Admin_Handler.cpp @@ -50,6 +50,9 @@ using json = nlohmann::json; // GenAI_Thread.h moved in Step 5. #include "SQLite3_Server.h" #include "Web_Interface.hpp" +#include "PgBouncer_Config.h" +#include "PgBouncer_ConfigConverter.h" +#include "PgBouncer_ShowCommands.h" #include #include @@ -5456,6 +5459,132 @@ void admin_session_handler(S* sess, void *_pa, PtrSize_t *pkt) { goto __run_query; } + // ========================================================================= + // IMPORT PGBOUNCER CONFIG FROM '/path/to/pgbouncer.ini' [DRY RUN] [IGNORE WARNINGS] + // ========================================================================= + if ((query_no_space_length > 30) && !strncasecmp("IMPORT PGBOUNCER CONFIG FROM ", query_no_space, 29)) { + proxy_info("Received %s command\n", query_no_space); + + // Parse command: extract file path and flags + std::string cmd_rest = std::string(query_no_space + 29); + bool dry_run = false; + bool ignore_warnings = false; + + // Check for DRY RUN and IGNORE WARNINGS flags (case-insensitive) + { + std::string upper_rest = cmd_rest; + std::transform(upper_rest.begin(), upper_rest.end(), upper_rest.begin(), ::toupper); + if (upper_rest.find("DRY RUN") != std::string::npos) { + dry_run = true; + auto pos = upper_rest.find("DRY RUN"); + cmd_rest.erase(pos, 7); + } + upper_rest = cmd_rest; + std::transform(upper_rest.begin(), upper_rest.end(), upper_rest.begin(), ::toupper); + if (upper_rest.find("IGNORE WARNINGS") != std::string::npos) { + ignore_warnings = true; + auto pos = upper_rest.find("IGNORE WARNINGS"); + cmd_rest.erase(pos, 15); + } + } + + char *path_buf = strdup(cmd_rest.c_str()); + char *file_path = trim_spaces_and_quotes_in_place(path_buf); + + // Stage 1: Parse PgBouncer config + PgBouncer::Config pgb_config; + bool parse_ok = PgBouncer::parse_config_file(std::string(file_path), pgb_config); + if (!parse_ok) { + std::string err_msg = "Failed to parse PgBouncer config: " + std::string(file_path); + for (const auto& e : pgb_config.errors) { + err_msg += "\n " + e.file + ":" + std::to_string(e.line) + ": " + e.message; + } + SPA->send_error_msg_to_client(sess, (char *)err_msg.c_str()); + free(path_buf); + run_query = false; + goto __run_query; + } + + // Stage 2: Convert + bool strict = !ignore_warnings; + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(pgb_config, strict); + + if (!result.success) { + std::string err_msg = "Conversion failed (unmappable parameters). Use IGNORE WARNINGS to proceed."; + for (const auto& e : result.errors) { + err_msg += "\n ERROR: " + e.message; + } + if (dry_run) { + err_msg += "\n\n" + PgBouncer::ConfigConverter::format_dry_run(result, file_path, strict); + } + SPA->send_error_msg_to_client(sess, (char *)err_msg.c_str()); + free(path_buf); + run_query = false; + goto __run_query; + } + + if (dry_run) { + std::string dry_output = PgBouncer::ConfigConverter::format_dry_run(result, file_path, strict); + std::string escaped = dry_output; + size_t pos = 0; + while ((pos = escaped.find('\'', pos)) != std::string::npos) { + escaped.replace(pos, 1, "''"); + pos += 2; + } + l_free(query_length, query); + std::string select_q = "SELECT '" + escaped + "' AS dry_run_output"; + query = l_strdup(select_q.c_str()); + query_length = strlen(query) + 1; + free(path_buf); + goto __run_query; + } + + // Execute: apply the conversion + for (const auto& entry : result.entries) { + if (!entry.sql.empty()) { + SPA->admindb->execute(entry.sql.c_str()); + } + } + + std::string ok_msg = "PgBouncer config imported: " + + std::to_string(result.server_count) + " servers, " + + std::to_string(result.user_count) + " users, " + + std::to_string(result.rule_count) + " query rules, " + + std::to_string(result.variable_count) + " variables"; + if (!result.warnings.empty()) { + ok_msg += " (" + std::to_string(result.warnings.size()) + " warnings)"; + } + SPA->send_ok_msg_to_client(sess, (char *)ok_msg.c_str(), 0, query_no_space); + free(path_buf); + run_query = false; + goto __run_query; + } + + // ========================================================================= + // PgBouncer-compatible SHOW commands (PgSQL admin port only) + // ========================================================================= + if constexpr (std::is_same_v) { + // Check for unsupported PgBouncer SHOW commands first + std::string unsupported_msg = PgBouncer::get_unsupported_show_message(query_no_space, query_no_space_length); + if (!unsupported_msg.empty()) { + SPA->send_error_msg_to_client(sess, (char *)unsupported_msg.c_str()); + run_query = false; + goto __run_query; + } + + // Try to translate PgBouncer SHOW commands + std::string translated_query; + bool is_extended = false; + if (PgBouncer::translate_show_command(query_no_space, query_no_space_length, + translated_query, is_extended)) { + l_free(query_length, query); + query = l_strdup(translated_query.c_str()); + query_length = strlen(query) + 1; + goto __run_query; + } + } + if (sess->session_type == PROXYSQL_SESSION_STATS) { // no admin if ( (strncasecmp("PRAGMA",query_no_space,6)==0) diff --git a/src/main.cpp b/src/main.cpp index 8aea072898..87f298cd9a 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2780,8 +2780,26 @@ void watchdog_main_loop() { } } +// Forward declaration for proxysql-cli mode +int proxysql_cli_main(int argc, const char* argv[]); + int main(int argc, const char * argv[]) { + // Detect proxysql-cli mode via argv[0] + { + const char* binary_name = argv[0]; + // Find the last path separator + const char* last_sep = strrchr(binary_name, '/'); + if (last_sep) binary_name = last_sep + 1; +#ifdef _WIN32 + const char* last_bsep = strrchr(binary_name, '\\'); + if (last_bsep && last_bsep > last_sep) binary_name = last_bsep + 1; +#endif + if (strcmp(binary_name, "proxysql-cli") == 0) { + return proxysql_cli_main(argc, argv); + } + } + if (check_openssl_version() == false) { exit(EXIT_FAILURE); } From abf16e368ffa45126337c691e57248c9658ec8b5 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 3 Apr 2026 14:16:19 +0000 Subject: [PATCH 04/12] ci: add GitHub Actions workflow for PgBouncer compat unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lightweight CI that builds and runs the three standalone PgBouncer compatibility unit tests (205 tests total) on every push to the feature branch and on PRs touching pgbouncer_compat files. No Docker or backend infrastructure needed — tests are standalone with zero ProxySQL runtime dependencies. --- .github/workflows/CI-pgbouncer-compat.yml | 50 +++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 .github/workflows/CI-pgbouncer-compat.yml diff --git a/.github/workflows/CI-pgbouncer-compat.yml b/.github/workflows/CI-pgbouncer-compat.yml new file mode 100644 index 0000000000..c44003ba08 --- /dev/null +++ b/.github/workflows/CI-pgbouncer-compat.yml @@ -0,0 +1,50 @@ +name: CI-pgbouncer-compat + +on: + push: + branches: [ 'feature/pgbouncer-compat' ] + paths: + - 'lib/pgbouncer_compat/**' + - 'include/PgBouncer_*.h' + - 'include/ProxySQL_CLI.h' + - 'test/tap/tests/unit/pgbouncer_*' + - '.github/workflows/CI-pgbouncer-compat.yml' + pull_request: + paths: + - 'lib/pgbouncer_compat/**' + - 'include/PgBouncer_*.h' + - 'include/ProxySQL_CLI.h' + - 'test/tap/tests/unit/pgbouncer_*' + +jobs: + unit-tests: + name: PgBouncer Compat Unit Tests + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Install dependencies + run: | + sudo apt-get update -qq + sudo apt-get install -y -qq build-essential g++ make + + - name: Build unit tests + working-directory: test/tap/tests/unit + run: | + make pgbouncer_config_parser_unit-t + make pgbouncer_converter_unit-t + make pgbouncer_show_commands_unit-t + + - name: Run parser tests (127 tests) + working-directory: test/tap/tests/unit + run: ./pgbouncer_config_parser_unit-t + + - name: Run converter tests (39 tests) + working-directory: test/tap/tests/unit + run: ./pgbouncer_converter_unit-t + + - name: Run SHOW commands tests (39 tests) + working-directory: test/tap/tests/unit + run: ./pgbouncer_show_commands_unit-t From 0fe1fbacc3836763eb1d0b2d7695087a29a9eb18 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 3 Apr 2026 14:17:30 +0000 Subject: [PATCH 05/12] ci: add libssl-dev to CI dependencies --- .github/workflows/CI-pgbouncer-compat.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CI-pgbouncer-compat.yml b/.github/workflows/CI-pgbouncer-compat.yml index c44003ba08..76f132bef7 100644 --- a/.github/workflows/CI-pgbouncer-compat.yml +++ b/.github/workflows/CI-pgbouncer-compat.yml @@ -28,7 +28,7 @@ jobs: - name: Install dependencies run: | sudo apt-get update -qq - sudo apt-get install -y -qq build-essential g++ make + sudo apt-get install -y -qq build-essential g++ make libssl-dev - name: Build unit tests working-directory: test/tap/tests/unit From 1f196cb3d90376b7baa7203470e4586115812f4b Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 3 Apr 2026 14:40:05 +0000 Subject: [PATCH 06/12] fix: address code review feedback from AI reviewers (#5566) Critical fixes: - Admin_Handler: Gate IMPORT PGBOUNCER CONFIG to admin sessions only (stats connections can no longer mutate config) - Admin_Handler: Wrap import execution in BEGIN/ROLLBACK for atomicity (partial failures no longer leave inconsistent state) - Admin_Handler: Parse DRY RUN/IGNORE WARNINGS as trailing tokens after quoted path (paths containing those strings no longer break parsing) - ProxySQL_CLI: Never write SQL to stdout on failed conversion (prevents partial SQL from being piped to mysql in error cases) Important fixes: - ConfigParser: Add INT_MIN/INT_MAX range checking in parse_int() and UINT_MAX checking in parse_uint() to prevent overflow - ConfigParser: Detect unterminated single quotes in connection string pairs and report parse error instead of silently accepting - ConfigParser: Include for range check constants - ConfigParser: Set file field in "cannot open" error message - ConfigConverter: DELETE FROM pgsql_firewall_whitelist_rules before importing HBA rules (prevents stale entries) - ShowCommands: Fix SHOW POOLS query to avoid cross-join that produced nondeterministic results with multiple pool rows - ProxySQL_CLI: Remove duplicate semicolons in non-dry-run output - Admin_Handler: Fix admindb->execute() return type (bool not int) --- lib/Admin_Handler.cpp | 107 ++++++++++++------ .../PgBouncer_ConfigConverter.cpp | 7 ++ .../PgBouncer_ConfigParser.cpp | 18 ++- .../PgBouncer_ShowCommands.cpp | 14 +-- lib/pgbouncer_compat/ProxySQL_CLI.cpp | 8 +- 5 files changed, 102 insertions(+), 52 deletions(-) diff --git a/lib/Admin_Handler.cpp b/lib/Admin_Handler.cpp index 51ee5b1044..0110632edf 100644 --- a/lib/Admin_Handler.cpp +++ b/lib/Admin_Handler.cpp @@ -5465,42 +5465,74 @@ void admin_session_handler(S* sess, void *_pa, PtrSize_t *pkt) { if ((query_no_space_length > 30) && !strncasecmp("IMPORT PGBOUNCER CONFIG FROM ", query_no_space, 29)) { proxy_info("Received %s command\n", query_no_space); - // Parse command: extract file path and flags + // Only allow from admin sessions, not stats + if (sess->session_type != PROXYSQL_SESSION_ADMIN) { + SPA->send_error_msg_to_client(sess, (char *)"IMPORT PGBOUNCER CONFIG requires admin privileges"); + run_query = false; + goto __run_query; + } + + // Parse flags as trailing tokens (after the quoted path). + // Syntax: IMPORT PGBOUNCER CONFIG FROM '/path' [DRY RUN] [IGNORE WARNINGS] + // Extract the quoted path first, then check remaining tokens for flags. std::string cmd_rest = std::string(query_no_space + 29); bool dry_run = false; bool ignore_warnings = false; - // Check for DRY RUN and IGNORE WARNINGS flags (case-insensitive) + // Find the file path (first quoted or unquoted token) + std::string file_path_str; + std::string remaining; { - std::string upper_rest = cmd_rest; - std::transform(upper_rest.begin(), upper_rest.end(), upper_rest.begin(), ::toupper); - if (upper_rest.find("DRY RUN") != std::string::npos) { + size_t start = cmd_rest.find_first_not_of(" \t"); + if (start == std::string::npos) { + SPA->send_error_msg_to_client(sess, (char *)"Missing file path"); + run_query = false; + goto __run_query; + } + if (cmd_rest[start] == '\'' || cmd_rest[start] == '"') { + char quote = cmd_rest[start]; + size_t end = cmd_rest.find(quote, start + 1); + if (end == std::string::npos) { + SPA->send_error_msg_to_client(sess, (char *)"Unterminated quote in file path"); + run_query = false; + goto __run_query; + } + file_path_str = cmd_rest.substr(start + 1, end - start - 1); + remaining = cmd_rest.substr(end + 1); + } else { + size_t end = cmd_rest.find_first_of(" \t", start); + if (end == std::string::npos) { + file_path_str = cmd_rest.substr(start); + } else { + file_path_str = cmd_rest.substr(start, end - start); + remaining = cmd_rest.substr(end); + } + } + } + + // Parse remaining tokens for flags + { + std::string upper_rem = remaining; + std::transform(upper_rem.begin(), upper_rem.end(), upper_rem.begin(), ::toupper); + // Remove extra whitespace for matching + // Check for "DRY RUN" and "IGNORE WARNINGS" as whole tokens + if (upper_rem.find("DRY RUN") != std::string::npos) { dry_run = true; - auto pos = upper_rest.find("DRY RUN"); - cmd_rest.erase(pos, 7); } - upper_rest = cmd_rest; - std::transform(upper_rest.begin(), upper_rest.end(), upper_rest.begin(), ::toupper); - if (upper_rest.find("IGNORE WARNINGS") != std::string::npos) { + if (upper_rem.find("IGNORE WARNINGS") != std::string::npos) { ignore_warnings = true; - auto pos = upper_rest.find("IGNORE WARNINGS"); - cmd_rest.erase(pos, 15); } } - char *path_buf = strdup(cmd_rest.c_str()); - char *file_path = trim_spaces_and_quotes_in_place(path_buf); - // Stage 1: Parse PgBouncer config PgBouncer::Config pgb_config; - bool parse_ok = PgBouncer::parse_config_file(std::string(file_path), pgb_config); + bool parse_ok = PgBouncer::parse_config_file(file_path_str, pgb_config); if (!parse_ok) { - std::string err_msg = "Failed to parse PgBouncer config: " + std::string(file_path); + std::string err_msg = "Failed to parse PgBouncer config: " + file_path_str; for (const auto& e : pgb_config.errors) { err_msg += "\n " + e.file + ":" + std::to_string(e.line) + ": " + e.message; } SPA->send_error_msg_to_client(sess, (char *)err_msg.c_str()); - free(path_buf); run_query = false; goto __run_query; } @@ -5516,16 +5548,15 @@ void admin_session_handler(S* sess, void *_pa, PtrSize_t *pkt) { err_msg += "\n ERROR: " + e.message; } if (dry_run) { - err_msg += "\n\n" + PgBouncer::ConfigConverter::format_dry_run(result, file_path, strict); + err_msg += "\n\n" + PgBouncer::ConfigConverter::format_dry_run(result, file_path_str.c_str(), strict); } SPA->send_error_msg_to_client(sess, (char *)err_msg.c_str()); - free(path_buf); run_query = false; goto __run_query; } if (dry_run) { - std::string dry_output = PgBouncer::ConfigConverter::format_dry_run(result, file_path, strict); + std::string dry_output = PgBouncer::ConfigConverter::format_dry_run(result, file_path_str.c_str(), strict); std::string escaped = dry_output; size_t pos = 0; while ((pos = escaped.find('\'', pos)) != std::string::npos) { @@ -5536,27 +5567,35 @@ void admin_session_handler(S* sess, void *_pa, PtrSize_t *pkt) { std::string select_q = "SELECT '" + escaped + "' AS dry_run_output"; query = l_strdup(select_q.c_str()); query_length = strlen(query) + 1; - free(path_buf); goto __run_query; } - // Execute: apply the conversion + // Execute atomically: wrap in transaction so partial failure rolls back + SPA->admindb->execute("BEGIN"); + bool exec_ok = true; for (const auto& entry : result.entries) { if (!entry.sql.empty()) { - SPA->admindb->execute(entry.sql.c_str()); + bool rc = SPA->admindb->execute(entry.sql.c_str()); + if (!rc) { + exec_ok = false; + SPA->admindb->execute("ROLLBACK"); + SPA->send_error_msg_to_client(sess, (char *)"Import failed during SQL execution; changes rolled back"); + break; + } } } - - std::string ok_msg = "PgBouncer config imported: " + - std::to_string(result.server_count) + " servers, " + - std::to_string(result.user_count) + " users, " + - std::to_string(result.rule_count) + " query rules, " + - std::to_string(result.variable_count) + " variables"; - if (!result.warnings.empty()) { - ok_msg += " (" + std::to_string(result.warnings.size()) + " warnings)"; + if (exec_ok) { + SPA->admindb->execute("COMMIT"); + std::string ok_msg = "PgBouncer config imported: " + + std::to_string(result.server_count) + " servers, " + + std::to_string(result.user_count) + " users, " + + std::to_string(result.rule_count) + " query rules, " + + std::to_string(result.variable_count) + " variables"; + if (!result.warnings.empty()) { + ok_msg += " (" + std::to_string(result.warnings.size()) + " warnings)"; + } + SPA->send_ok_msg_to_client(sess, (char *)ok_msg.c_str(), 0, query_no_space); } - SPA->send_ok_msg_to_client(sess, (char *)ok_msg.c_str(), 0, query_no_space); - free(path_buf); run_query = false; goto __run_query; } diff --git a/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp b/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp index 57b92005f1..17a0bb3b8c 100644 --- a/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp +++ b/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp @@ -375,6 +375,13 @@ void ConfigConverter::convert_hba_rules(const Config& config, bool any_converted = false; + if (!config.hba_rules.empty()) { + result.entries.push_back({ + "DELETE FROM pgsql_firewall_whitelist_rules;", + "Remove existing firewall whitelist rules before importing HBA rules" + }); + } + for (const auto& rule : config.hba_rules) { // Unsupported connection types if (rule.conn_type == "local") { diff --git a/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp b/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp index 54852069ba..5aafb72a51 100644 --- a/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp +++ b/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp @@ -4,6 +4,7 @@ #include #include #include +#include namespace PgBouncer { @@ -57,6 +58,7 @@ bool ConfigParser::parse_int(const std::string& value, int& result) { size_t pos = 0; long v = std::stol(value, &pos); if (pos != value.size()) return false; + if (v < INT_MIN || v > INT_MAX) return false; result = static_cast(v); return true; } catch (...) { @@ -70,6 +72,7 @@ bool ConfigParser::parse_uint(const std::string& value, unsigned int& result) { size_t pos = 0; unsigned long v = std::stoul(value, &pos); if (pos != value.size()) return false; + if (v > UINT_MAX) return false; result = static_cast(v); return true; } catch (...) { @@ -117,24 +120,27 @@ bool ConfigParser::parse_connstr_pairs( // Quoted value: collect until unescaped closing quote // PgBouncer escapes single quotes by doubling: '' ++i; // skip opening quote - std::string raw = "'"; + bool closed = false; while (i < len) { if (connstr[i] == '\'') { if (i + 1 < len && connstr[i + 1] == '\'') { - raw += "''"; + value += '\''; i += 2; } else { // closing quote ++i; + closed = true; break; } } else { - raw += connstr[i]; + value += connstr[i]; ++i; } } - raw += "'"; - value = unquote(raw); + if (!closed) { + errors.push_back({file, line, "unterminated single quote in connection string value for key '" + key + "'"}); + return false; + } } else { // Unquoted value: read until whitespace size_t val_start = i; @@ -503,7 +509,7 @@ bool ConfigParser::parse_ini( { std::ifstream ifs(filepath); if (!ifs.is_open()) { - config.errors.push_back({"", 0, "cannot open file: " + filepath}); + config.errors.push_back({filepath, 0, "cannot open file: " + filepath}); return false; } diff --git a/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp b/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp index 09aa490744..2e25e65f6f 100644 --- a/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp +++ b/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp @@ -71,8 +71,8 @@ static std::vector tokenize_upper(const std::string& s) { static std::string query_pools(bool extended) { std::string q = "SELECT " - "'default' AS database, " - "su.username AS user, " + "cp.srv_host AS database, " + "'-' AS user, " "0 AS cl_active, " "0 AS cl_waiting, " "0 AS cl_cancel_req, " @@ -83,20 +83,16 @@ static std::string query_pools(bool extended) { "0 AS sv_login, " "0 AS maxwait, " "0 AS maxwait_us, " - "CASE WHEN su.fast_forward=1 THEN 'session' " - "WHEN su.transaction_persistent=1 THEN 'transaction' " - "ELSE 'statement' END AS pool_mode"; + "'statement' AS pool_mode"; if (extended) { q += ", cp.hostgroup AS hostgroup_id" - ", cp.ConnUsed AS multiplex" + ", 1 AS multiplex" ", cp.Latency_us AS latency_us" ", cp.Queries AS Queries" ", cp.Bytes_data_sent AS Bytes_data_sent" ", cp.Bytes_data_recv AS Bytes_data_recv"; } - q += " FROM stats_pgsql_connection_pool cp" - " JOIN runtime_pgsql_users su ON 1=1" - " GROUP BY su.username"; + q += " FROM stats_pgsql_connection_pool cp"; return q; } diff --git a/lib/pgbouncer_compat/ProxySQL_CLI.cpp b/lib/pgbouncer_compat/ProxySQL_CLI.cpp index fe6e358291..c31af5abb9 100644 --- a/lib/pgbouncer_compat/ProxySQL_CLI.cpp +++ b/lib/pgbouncer_compat/ProxySQL_CLI.cpp @@ -68,8 +68,10 @@ static int cmd_import_pgbouncer(int argc, const char* argv[]) { for (const auto& err : result.errors) { std::cerr << " ERROR: " << err.message << "\n"; } - // Still show the dry-run output so the user can see what would have been converted - std::cout << PgBouncer::ConfigConverter::format_dry_run(result, config_path, strict); + // Show dry-run output on stderr only (never stdout — it could be piped to mysql) + if (dry_run) { + std::cerr << "\n" << PgBouncer::ConfigConverter::format_dry_run(result, config_path, strict); + } return 1; } @@ -98,7 +100,7 @@ static int cmd_import_pgbouncer(int argc, const char* argv[]) { if (!entry.comment.empty()) { std::cout << "-- " << entry.comment << "\n"; } - std::cout << entry.sql << ";\n"; + std::cout << entry.sql << "\n"; } if (!result.warnings.empty()) { From 087c7851cf8ea2e8bcee50c8c1aacb9ceb306feb Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Thu, 27 Aug 2026 06:08:11 +0000 Subject: [PATCH 07/12] fix(pgbouncer): correct generated SQL, SHOW ordering, and parser quoting The generated SQL and several SHOW translations referenced columns that do not exist, so they failed the moment they were executed. The unit tests only compared strings, which is why all 205 of them passed against broken SQL. Generated SQL: - pgsql_query_rules has no `schemaname` column (that is the MySQL table); the PgSQL one calls it `database`. Every routing rule INSERT was invalid. - pgsql_firewall_whitelist_rules likewise uses `database`, and declares `digest` and `comment` NOT NULL with no default, so the INSERT was invalid. SHOW translations: - SHOW CLIENTS selected `db` from stats_pgsql_processlist; the column is `database`. - SHOW DATABASES and SHOW EXTENDED SERVERS read weight, max_connections and max_replication_lag from stats_pgsql_connection_pool, which has none of them. They now read (or join) runtime_pgsql_servers. - SHOW DATABASES was unreachable: Admin_Handler's generic SHOW block claims that command word first. The PgBouncer block now runs before it, and translate_show_command() rejects trailing tokens so everything else still falls through. - SHOW EXTENDED was a no-op for eight of the ten commands. Semantics that were silently wrong: - convert_users() dropped AuthFileEntry::type and wrote MD5/SCRAM verifiers into pgsql_users.password. ProxySQL derives both the MD5 response and the SCRAM verifier from the cleartext password, so those credentials could never authenticate. Now reported per user (fatal in strict mode). - A `dbname=` alias was dropped, emitting a rule that routes to the hostgroup while passing the client's database name through unchanged. - HBA `reject` rules were emitted as an SQL comment and still enabled the whitelist, implying a denial that the allow-only whitelist cannot express. Parser: - Inline-comment stripping is quote-aware, and quoted [pgbouncer] values are unquoted. Previously any value starting with a quote skipped stripping entirely and kept its quotes. - The HBA tokenizer handles doubled "" escapes, preserves empty quoted tokens instead of shifting later fields, and reports an unterminated quote instead of returning truncated tokens. - parse() is a full load: re-parsing into the same Config no longer duplicates every database, user and rule. - parse_uint() rejects a leading '-' rather than relying on stoul wraparound. Also removes the six duplicated headers under lib/pgbouncer_compat/ (identical copies of the include/ ones) and switches the include guards to the project's __CLASS_*_H convention. Unit tests: 205 -> 256, with a regression test for each defect above. --- include/PgBouncer_AuthFileParser.h | 6 +- include/PgBouncer_Config.h | 6 +- include/PgBouncer_ConfigConverter.h | 10 +- include/PgBouncer_ConfigParser.h | 7 +- include/PgBouncer_HBAParser.h | 12 +- include/PgBouncer_ShowCommands.h | 6 +- include/ProxySQL_CLI.h | 6 +- lib/Admin_Handler.cpp | 55 ++--- .../PgBouncer_AuthFileParser.cpp | 2 + .../PgBouncer_AuthFileParser.h | 35 --- .../PgBouncer_ConfigConverter.cpp | 93 +++++--- .../PgBouncer_ConfigConverter.h | 64 ------ .../PgBouncer_ConfigParser.cpp | 51 +++-- lib/pgbouncer_compat/PgBouncer_ConfigParser.h | 57 ----- lib/pgbouncer_compat/PgBouncer_HBAParser.cpp | 39 +++- lib/pgbouncer_compat/PgBouncer_HBAParser.h | 37 ---- .../PgBouncer_ShowCommands.cpp | 200 ++++++++++++------ lib/pgbouncer_compat/PgBouncer_ShowCommands.h | 22 -- lib/pgbouncer_compat/ProxySQL_CLI.cpp | 24 ++- lib/pgbouncer_compat/ProxySQL_CLI.h | 9 - .../pgbouncer_compat/hba_quoting.conf | 4 + .../pgbouncer_compat/hba_unterminated.conf | 1 + .../fixtures/pgbouncer_compat/quoting.ini | 15 ++ .../unit/pgbouncer_config_parser_unit-t.cpp | 118 ++++++++++- .../tests/unit/pgbouncer_converter_unit-t.cpp | 175 ++++++++++++++- .../unit/pgbouncer_show_commands_unit-t.cpp | 104 +++++++-- 26 files changed, 763 insertions(+), 395 deletions(-) delete mode 100644 lib/pgbouncer_compat/PgBouncer_AuthFileParser.h delete mode 100644 lib/pgbouncer_compat/PgBouncer_ConfigConverter.h delete mode 100644 lib/pgbouncer_compat/PgBouncer_ConfigParser.h delete mode 100644 lib/pgbouncer_compat/PgBouncer_HBAParser.h delete mode 100644 lib/pgbouncer_compat/PgBouncer_ShowCommands.h delete mode 100644 lib/pgbouncer_compat/ProxySQL_CLI.h create mode 100644 test/tap/tests/unit/fixtures/pgbouncer_compat/hba_quoting.conf create mode 100644 test/tap/tests/unit/fixtures/pgbouncer_compat/hba_unterminated.conf create mode 100644 test/tap/tests/unit/fixtures/pgbouncer_compat/quoting.ini diff --git a/include/PgBouncer_AuthFileParser.h b/include/PgBouncer_AuthFileParser.h index 29f27f15bb..4b7853c5f5 100644 --- a/include/PgBouncer_AuthFileParser.h +++ b/include/PgBouncer_AuthFileParser.h @@ -1,5 +1,5 @@ -#ifndef PGBOUNCER_AUTH_FILE_PARSER_H -#define PGBOUNCER_AUTH_FILE_PARSER_H +#ifndef __CLASS_PGBOUNCER_AUTH_FILE_PARSER_H +#define __CLASS_PGBOUNCER_AUTH_FILE_PARSER_H #include "PgBouncer_Config.h" #include @@ -32,4 +32,4 @@ class AuthFileParser { } // namespace PgBouncer -#endif +#endif // __CLASS_PGBOUNCER_AUTH_FILE_PARSER_H diff --git a/include/PgBouncer_Config.h b/include/PgBouncer_Config.h index 34e151f1e1..cc62c515fa 100644 --- a/include/PgBouncer_Config.h +++ b/include/PgBouncer_Config.h @@ -1,5 +1,5 @@ -#ifndef PGBOUNCER_CONFIG_H -#define PGBOUNCER_CONFIG_H +#ifndef __CLASS_PGBOUNCER_CONFIG_H +#define __CLASS_PGBOUNCER_CONFIG_H #include #include @@ -221,4 +221,4 @@ bool parse_hba_file(const std::string& filepath, std::vector& rules, st } // namespace PgBouncer -#endif // PGBOUNCER_CONFIG_H +#endif // __CLASS_PGBOUNCER_CONFIG_H diff --git a/include/PgBouncer_ConfigConverter.h b/include/PgBouncer_ConfigConverter.h index 33aa24226e..f1ddf9a199 100644 --- a/include/PgBouncer_ConfigConverter.h +++ b/include/PgBouncer_ConfigConverter.h @@ -1,5 +1,5 @@ -#ifndef PGBOUNCER_CONFIG_CONVERTER_H -#define PGBOUNCER_CONFIG_CONVERTER_H +#ifndef __CLASS_PGBOUNCER_CONFIG_CONVERTER_H +#define __CLASS_PGBOUNCER_CONFIG_CONVERTER_H #include "PgBouncer_Config.h" #include @@ -42,8 +42,8 @@ class ConfigConverter { int next_rule_id_ = 1; int wildcard_hostgroup_ = -1; // hostgroup for the * database, or -1 - void convert_databases(const Config& config, ConversionResult& result); - void convert_users(const Config& config, ConversionResult& result); + void convert_databases(const Config& config, ConversionResult& result, bool strict); + void convert_users(const Config& config, ConversionResult& result, bool strict); void convert_globals(const Config& config, ConversionResult& result, bool strict); void convert_hba_rules(const Config& config, ConversionResult& result, bool strict); void add_load_and_save(ConversionResult& result); @@ -61,4 +61,4 @@ class ConfigConverter { } // namespace PgBouncer -#endif +#endif // __CLASS_PGBOUNCER_CONFIG_CONVERTER_H diff --git a/include/PgBouncer_ConfigParser.h b/include/PgBouncer_ConfigParser.h index 521f0a57a1..80b04ecde7 100644 --- a/include/PgBouncer_ConfigParser.h +++ b/include/PgBouncer_ConfigParser.h @@ -1,5 +1,5 @@ -#ifndef PGBOUNCER_CONFIG_PARSER_H -#define PGBOUNCER_CONFIG_PARSER_H +#ifndef __CLASS_PGBOUNCER_CONFIG_PARSER_H +#define __CLASS_PGBOUNCER_CONFIG_PARSER_H #include "PgBouncer_Config.h" #include @@ -47,6 +47,7 @@ class ConfigParser { // String utilities static std::string trim(const std::string& s); static std::string unquote(const std::string& s); + static std::string strip_inline_comment(const std::string& s); static bool parse_bool(const std::string& value, bool& result); static bool parse_int(const std::string& value, int& result); static bool parse_uint(const std::string& value, unsigned int& result); @@ -54,4 +55,4 @@ class ConfigParser { } // namespace PgBouncer -#endif +#endif // __CLASS_PGBOUNCER_CONFIG_PARSER_H diff --git a/include/PgBouncer_HBAParser.h b/include/PgBouncer_HBAParser.h index 6d3fc373ed..48cae4fd90 100644 --- a/include/PgBouncer_HBAParser.h +++ b/include/PgBouncer_HBAParser.h @@ -1,5 +1,5 @@ -#ifndef PGBOUNCER_HBA_PARSER_H -#define PGBOUNCER_HBA_PARSER_H +#ifndef __CLASS_PGBOUNCER_HBA_PARSER_H +#define __CLASS_PGBOUNCER_HBA_PARSER_H #include "PgBouncer_Config.h" #include @@ -22,8 +22,10 @@ class HBAParser { std::vector& errors); private: - // Tokenize a line respecting double-quoted strings - static std::vector tokenize(const std::string& line); + // Tokenize a line respecting double-quoted strings (a doubled "" inside a + // quoted run is a literal quote). Returns false if a quote is left open, + // in which case the tokens are incomplete and must not be used. + static bool tokenize(const std::string& line, std::vector& tokens); // Parse a single HBA record from tokens bool parse_record(const std::vector& tokens, @@ -34,4 +36,4 @@ class HBAParser { } // namespace PgBouncer -#endif +#endif // __CLASS_PGBOUNCER_HBA_PARSER_H diff --git a/include/PgBouncer_ShowCommands.h b/include/PgBouncer_ShowCommands.h index f6018f213f..b7f5cbaad8 100644 --- a/include/PgBouncer_ShowCommands.h +++ b/include/PgBouncer_ShowCommands.h @@ -1,5 +1,5 @@ -#ifndef PGBOUNCER_SHOW_COMMANDS_H -#define PGBOUNCER_SHOW_COMMANDS_H +#ifndef __CLASS_PGBOUNCER_SHOW_COMMANDS_H +#define __CLASS_PGBOUNCER_SHOW_COMMANDS_H #include @@ -19,4 +19,4 @@ std::string get_unsupported_show_message(const char* query, int query_len); } // namespace PgBouncer -#endif +#endif // __CLASS_PGBOUNCER_SHOW_COMMANDS_H diff --git a/include/ProxySQL_CLI.h b/include/ProxySQL_CLI.h index 8853f453fe..34b10b144a 100644 --- a/include/ProxySQL_CLI.h +++ b/include/ProxySQL_CLI.h @@ -1,9 +1,9 @@ -#ifndef PROXYSQL_CLI_H -#define PROXYSQL_CLI_H +#ifndef __CLASS_PROXYSQL_CLI_H +#define __CLASS_PROXYSQL_CLI_H // Entry point for proxysql-cli mode. // Called when argv[0] ends with "proxysql-cli". // Returns the exit code (0 success, 1 error). int proxysql_cli_main(int argc, const char* argv[]); -#endif +#endif // __CLASS_PROXYSQL_CLI_H diff --git a/lib/Admin_Handler.cpp b/lib/Admin_Handler.cpp index 0110632edf..230149ed84 100644 --- a/lib/Admin_Handler.cpp +++ b/lib/Admin_Handler.cpp @@ -5128,6 +5128,37 @@ void admin_session_handler(S* sess, void *_pa, PtrSize_t *pkt) { } } + // ========================================================================= + // PgBouncer-compatible SHOW commands (PgSQL admin port only) + // + // This must run BEFORE the generic SHOW dispatch below: ProxySQL already + // owns some of the same command words (notably `SHOW DATABASES`), so a + // translation placed after that block would never be reached for them. + // translate_show_command() only claims the exact PgBouncer command set and + // rejects anything with trailing tokens, so every other SHOW still falls + // through to the normal handling. + // ========================================================================= + if constexpr (std::is_same_v) { + // Check for unsupported PgBouncer SHOW commands first + std::string unsupported_msg = PgBouncer::get_unsupported_show_message(query_no_space, query_no_space_length); + if (!unsupported_msg.empty()) { + SPA->send_error_msg_to_client(sess, (char *)unsupported_msg.c_str()); + run_query = false; + goto __run_query; + } + + // Try to translate PgBouncer SHOW commands + std::string translated_query; + bool is_extended = false; + if (PgBouncer::translate_show_command(query_no_space, query_no_space_length, + translated_query, is_extended)) { + l_free(query_length, query); + query = l_strdup(translated_query.c_str()); + query_length = strlen(query) + 1; + goto __run_query; + } + } + if (strncasecmp("SHOW ", query_no_space, 5)) { goto __end_show_commands; // in the next block there are only SHOW commands } @@ -5600,30 +5631,6 @@ void admin_session_handler(S* sess, void *_pa, PtrSize_t *pkt) { goto __run_query; } - // ========================================================================= - // PgBouncer-compatible SHOW commands (PgSQL admin port only) - // ========================================================================= - if constexpr (std::is_same_v) { - // Check for unsupported PgBouncer SHOW commands first - std::string unsupported_msg = PgBouncer::get_unsupported_show_message(query_no_space, query_no_space_length); - if (!unsupported_msg.empty()) { - SPA->send_error_msg_to_client(sess, (char *)unsupported_msg.c_str()); - run_query = false; - goto __run_query; - } - - // Try to translate PgBouncer SHOW commands - std::string translated_query; - bool is_extended = false; - if (PgBouncer::translate_show_command(query_no_space, query_no_space_length, - translated_query, is_extended)) { - l_free(query_length, query); - query = l_strdup(translated_query.c_str()); - query_length = strlen(query) + 1; - goto __run_query; - } - } - if (sess->session_type == PROXYSQL_SESSION_STATS) { // no admin if ( (strncasecmp("PRAGMA",query_no_space,6)==0) diff --git a/lib/pgbouncer_compat/PgBouncer_AuthFileParser.cpp b/lib/pgbouncer_compat/PgBouncer_AuthFileParser.cpp index 74a27ff44d..dc59561f99 100644 --- a/lib/pgbouncer_compat/PgBouncer_AuthFileParser.cpp +++ b/lib/pgbouncer_compat/PgBouncer_AuthFileParser.cpp @@ -76,6 +76,8 @@ AuthType AuthFileParser::detect_password_type(const std::string& password) { bool AuthFileParser::parse(const std::string& filepath, std::vector& entries, std::vector& errors) { + // Full load, not an append -- see the note in HBAParser::parse. + entries.clear(); std::ifstream file(filepath); if (!file.is_open()) { errors.push_back({filepath, 0, "Cannot open auth file: " + filepath}); diff --git a/lib/pgbouncer_compat/PgBouncer_AuthFileParser.h b/lib/pgbouncer_compat/PgBouncer_AuthFileParser.h deleted file mode 100644 index 29f27f15bb..0000000000 --- a/lib/pgbouncer_compat/PgBouncer_AuthFileParser.h +++ /dev/null @@ -1,35 +0,0 @@ -#ifndef PGBOUNCER_AUTH_FILE_PARSER_H -#define PGBOUNCER_AUTH_FILE_PARSER_H - -#include "PgBouncer_Config.h" -#include -#include - -namespace PgBouncer { - -class AuthFileParser { -public: - // Parse a PgBouncer userlist.txt file. - // Format: "username" "password" per line - // Password types detected: - // - Plain text: any string not matching MD5 or SCRAM patterns - // - MD5: starts with "md5" followed by 32 hex chars - // - SCRAM: starts with "SCRAM-SHA-256$" - // Double-quote escaping: "" inside quoted strings represents a literal " - bool parse(const std::string& filepath, - std::vector& entries, - std::vector& errors); - -private: - // Parse a double-quoted string starting at pos, advancing pos past the closing quote. - // Returns the unescaped content. Returns false if malformed. - static bool parse_quoted_string(const std::string& line, size_t& pos, - std::string& result); - - // Detect password type from the raw password string - static AuthType detect_password_type(const std::string& password); -}; - -} // namespace PgBouncer - -#endif diff --git a/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp b/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp index 17a0bb3b8c..43e8401ec6 100644 --- a/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp +++ b/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp @@ -44,8 +44,8 @@ ConversionResult ConfigConverter::convert(const Config& config, bool strict) { ConversionResult result; - convert_databases(config, result); - convert_users(config, result); + convert_databases(config, result, strict); + convert_users(config, result, strict); convert_globals(config, result, strict); convert_hba_rules(config, result, strict); check_unmappable(config, result, strict); @@ -75,7 +75,7 @@ static std::vector split(const std::string& s, char delim) { // convert_databases // --------------------------------------------------------------------------- void ConfigConverter::convert_databases(const Config& config, - ConversionResult& result) { + ConversionResult& result, bool strict) { if (config.databases.empty()) return; // Clean slate @@ -135,22 +135,34 @@ void ConfigConverter::convert_databases(const Config& config, if (db.name != "*") { int rule_id = next_rule_id_++; - std::string dest_db = db.dbname.empty() ? db.name : db.dbname; - + // The routing column in pgsql_query_rules is `database` (the + // MySQL table calls it `schemaname`; the PgSQL one does not). std::ostringstream sql; sql << "INSERT INTO pgsql_query_rules " - << "(rule_id, active, schemaname, destination_hostgroup, apply) " + << "(rule_id, active, database, destination_hostgroup, apply) " << "VALUES (" << rule_id << ", 1, " << "'" << sql_escape(db.name) << "', " << hg << ", 1);"; std::string comment = "Route database '" + db.name + "' to hostgroup " + std::to_string(hg); - if (db.dbname != "" && db.dbname != db.name) - comment += " (backend db: " + db.dbname + ")"; result.entries.push_back({sql.str(), comment}); result.rule_count++; + + // `dbname=` makes PgBouncer connect to a backend database under a + // different name than the one the client asked for. ProxySQL routes + // to a hostgroup but does not rewrite the database in the startup + // packet, so the alias cannot be honoured -- say so instead of + // emitting a rule that quietly connects to the wrong database. + if (!db.dbname.empty() && db.dbname != db.name) { + add_issue(result, strict, + "database '" + db.name + "' maps to backend database '" + + db.dbname + "' (dbname=), which ProxySQL cannot rewrite; " + "rule_id " + std::to_string(rule_id) + + " routes to the hostgroup but the backend database name is " + "passed through unchanged"); + } } } } @@ -159,11 +171,15 @@ void ConfigConverter::convert_databases(const Config& config, // convert_users // --------------------------------------------------------------------------- void ConfigConverter::convert_users(const Config& config, - ConversionResult& result) { - // Build a password lookup from auth_entries - std::map passwords; + ConversionResult& result, bool strict) { + // Build a password lookup from auth_entries, keeping the detected type. + // The type matters: ProxySQL derives both the MD5 challenge response and + // the SCRAM verifier from the *cleartext* password in pgsql_users.password + // (see PgSQL_Protocol.cpp), so a pre-hashed userlist.txt entry cannot be + // imported as a working credential. + std::map passwords; for (const auto& ae : config.auth_entries) { - passwords[ae.username] = ae.password; + passwords[ae.username] = &ae; } // Collect users from [users] section; also add any auth_entries users not @@ -194,7 +210,19 @@ void ConfigConverter::convert_users(const Config& config, // Resolve password from auth_entries std::string password; auto it = passwords.find(u.name); - if (it != passwords.end()) password = it->second; + if (it != passwords.end()) { + password = it->second->password; + if (it->second->type != AuthType::PLAIN) { + const char* kind = + (it->second->type == AuthType::MD5) ? "MD5" : "SCRAM-SHA-256"; + add_issue(result, strict, + "user '" + u.name + "' has a " + kind + " verifier in the " + "auth file; ProxySQL needs the cleartext password in " + "pgsql_users.password to answer PostgreSQL authentication, " + "so this credential is imported verbatim but will not " + "authenticate until it is replaced with the cleartext value"); + } + } // Pool mode mapping std::string pool = u.pool_mode.empty() ? config.global.pool_mode : u.pool_mode; @@ -322,7 +350,9 @@ void ConfigConverter::convert_globals(const Config& config, g.server_tls_sslmode == "verify-ca" || g.server_tls_sslmode == "verify-full"); if (need_ssl) { - // Update all previously inserted server rows to use_ssl=1 + // Safe to apply unscoped: convert_databases() emits + // "DELETE FROM pgsql_servers" before its INSERTs, so every row in + // the table at this point came from this import. result.entries.push_back({ "UPDATE pgsql_servers SET use_ssl=1;", "PgBouncer server_tls_sslmode=" + g.server_tls_sslmode + @@ -425,14 +455,15 @@ void ConfigConverter::convert_hba_rules(const Config& config, // absence from the whitelist effectively blocks access when whitelist mode // is enabled; we emit a comment explaining this) if (rule.method == "reject") { - result.entries.push_back({ - "-- HBA reject rule: " + rule.conn_type + " " + rule.database + - " " + rule.user + " " + addr + " reject", - "ProxySQL firewall whitelist is allow-only; not adding this " - "source/user means traffic from " + addr + " is implicitly denied " - "when pgsql-firewall_whitelist_enabled=1" - }); - any_converted = true; + // The ProxySQL whitelist is allow-only and has no deny entry. An + // HBA `reject` that precedes a broader allow rule therefore cannot + // be reproduced: the allow would win. Surface it rather than + // emitting a comment and implying the deny was handled. + add_issue(result, strict, + "HBA 'reject' rule (" + rule.conn_type + " " + rule.database + + " " + rule.user + " " + addr + ") cannot be represented: " + "the ProxySQL firewall whitelist is allow-only, so this " + "denial must be reproduced manually or enforced upstream"); continue; } @@ -443,18 +474,24 @@ void ConfigConverter::convert_hba_rules(const Config& config, std::string user_val = (rule.user == "all") ? "" : rule.user; std::string db_val = (rule.database == "all") ? "" : rule.database; + std::string comment = "HBA allow: " + rule.conn_type + " " + + rule.database + " " + rule.user + " " + + addr + " " + rule.method; + + // The column is `database` (not `schemaname`), and both `digest` + // and `comment` are NOT NULL without a default, so they must be + // supplied explicitly. An empty digest whitelists every query for + // the (user, address, database) triple, which is the closest + // equivalent to an HBA allow rule. std::ostringstream sql; sql << "INSERT INTO pgsql_firewall_whitelist_rules " - << "(active, client_address, username, schemaname, flagIN) " + << "(active, client_address, username, database, flagIN, digest, comment) " << "VALUES (1, " << "'" << sql_escape(addr) << "', " << "'" << sql_escape(user_val) << "', " << "'" << sql_escape(db_val) << "', " - << "0);"; - - std::string comment = "HBA allow: " + rule.conn_type + " " + - rule.database + " " + rule.user + " " + - addr + " " + rule.method; + << "0, '', " + << "'" << sql_escape(comment) << "');"; result.entries.push_back({sql.str(), comment}); any_converted = true; diff --git a/lib/pgbouncer_compat/PgBouncer_ConfigConverter.h b/lib/pgbouncer_compat/PgBouncer_ConfigConverter.h deleted file mode 100644 index 33aa24226e..0000000000 --- a/lib/pgbouncer_compat/PgBouncer_ConfigConverter.h +++ /dev/null @@ -1,64 +0,0 @@ -#ifndef PGBOUNCER_CONFIG_CONVERTER_H -#define PGBOUNCER_CONFIG_CONVERTER_H - -#include "PgBouncer_Config.h" -#include -#include - -namespace PgBouncer { - -struct ConversionEntry { - std::string sql; // SQL statement - std::string comment; // Explanatory comment -}; - -struct ConversionResult { - std::vector entries; - std::vector warnings; // Non-fatal mapping issues - std::vector errors; // Unmappable parameters (strict mode) - bool success = true; - - // Summary counts - int server_count = 0; - int user_count = 0; - int rule_count = 0; - int variable_count = 0; -}; - -class ConfigConverter { -public: - // Convert a parsed PgBouncer config into ProxySQL SQL statements. - // If strict is true (default), unmappable parameters produce errors and success=false. - // If strict is false, unmappable parameters produce warnings only. - ConversionResult convert(const Config& config, bool strict = true); - - // Generate the full dry-run output as a string (SQL with comments) - static std::string format_dry_run(const ConversionResult& result, - const std::string& source_path, - bool strict); - -private: - int next_hostgroup_ = 0; - int next_rule_id_ = 1; - int wildcard_hostgroup_ = -1; // hostgroup for the * database, or -1 - - void convert_databases(const Config& config, ConversionResult& result); - void convert_users(const Config& config, ConversionResult& result); - void convert_globals(const Config& config, ConversionResult& result, bool strict); - void convert_hba_rules(const Config& config, ConversionResult& result, bool strict); - void add_load_and_save(ConversionResult& result); - - // Check for unmappable parameters - void check_unmappable(const Config& config, ConversionResult& result, bool strict); - - // Helper to add error or warning based on strict mode - void add_issue(ConversionResult& result, bool strict, - const std::string& msg); - - // SQL escaping for string values - static std::string sql_escape(const std::string& s); -}; - -} // namespace PgBouncer - -#endif diff --git a/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp b/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp index 5aafb72a51..03d73a3ba2 100644 --- a/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp +++ b/lib/pgbouncer_compat/PgBouncer_ConfigParser.cpp @@ -37,6 +37,30 @@ std::string ConfigParser::unquote(const std::string& s) { return s; } +std::string ConfigParser::strip_inline_comment(const std::string& s) { + // Truncate at the first '#' or ';' that sits outside a single-quoted run and + // is preceded by whitespace (or starts the value). PgBouncer escapes a quote + // inside a quoted value by doubling it (''). + bool in_quote = false; + for (size_t i = 0; i < s.size(); ++i) { + if (s[i] == '\'') { + if (in_quote && i + 1 < s.size() && s[i + 1] == '\'') { + ++i; // escaped quote, stays inside the quoted run + continue; + } + in_quote = !in_quote; + continue; + } + if (in_quote) continue; + if (s[i] == '#' || s[i] == ';') { + if (i == 0 || std::isspace(static_cast(s[i - 1]))) { + return trim(s.substr(0, i)); + } + } + } + return s; +} + bool ConfigParser::parse_bool(const std::string& value, bool& result) { std::string lower = value; std::transform(lower.begin(), lower.end(), lower.begin(), @@ -68,11 +92,14 @@ bool ConfigParser::parse_int(const std::string& value, int& result) { bool ConfigParser::parse_uint(const std::string& value, unsigned int& result) { if (value.empty()) return false; + // std::stoul wraps a negative literal around instead of throwing, so reject + // the sign up front rather than relying on the range check below. + if (value[0] == '-') return false; try { size_t pos = 0; unsigned long v = std::stoul(value, &pos); if (pos != value.size()) return false; - if (v > UINT_MAX) return false; + if (v > static_cast(UINT_MAX)) return false; result = static_cast(v); return true; } catch (...) { @@ -594,18 +621,14 @@ bool ConfigParser::parse_ini( std::string key = trim(trimmed.substr(0, eq_pos)); std::string value = trim(trimmed.substr(eq_pos + 1)); - // Strip inline comments from values (only for [pgbouncer] section, not connection strings) + // Strip inline comments from values (only for [pgbouncer] section; the + // other sections hold connection strings, parsed by parse_connstr_pairs). + // A '#' or ';' inside a quoted value is data, not a comment. if (current_section == Section::PGBOUNCER) { - // Remove trailing comments, but be careful with quoted values - if (!value.empty() && value[0] != '\'') { - auto comment_pos = value.find(" #"); - if (comment_pos == std::string::npos) comment_pos = value.find(" ;"); - if (comment_pos == std::string::npos) comment_pos = value.find("\t#"); - if (comment_pos == std::string::npos) comment_pos = value.find("\t;"); - if (comment_pos != std::string::npos) { - value = trim(value.substr(0, comment_pos)); - } - } + value = strip_inline_comment(value); + // PgBouncer allows quoting a global value to protect spaces and + // comment characters; the quotes are not part of the value. + value = unquote(value); } if (current_section == Section::NONE) { @@ -687,6 +710,10 @@ bool ConfigParser::parse( const std::string& filepath, Config& config, bool resolve_includes, bool resolve_referenced_files) { + // Start from a clean slate: parse() is a full load, not an append. Without + // this, reusing a Config (or a ConfigParser) across calls duplicates every + // database/user/rule and keeps stale errors alive. + config = Config(); include_depth_ = 0; return parse_ini(filepath, config, resolve_includes, resolve_referenced_files); } diff --git a/lib/pgbouncer_compat/PgBouncer_ConfigParser.h b/lib/pgbouncer_compat/PgBouncer_ConfigParser.h deleted file mode 100644 index 521f0a57a1..0000000000 --- a/lib/pgbouncer_compat/PgBouncer_ConfigParser.h +++ /dev/null @@ -1,57 +0,0 @@ -#ifndef PGBOUNCER_CONFIG_PARSER_H -#define PGBOUNCER_CONFIG_PARSER_H - -#include "PgBouncer_Config.h" -#include -#include - -namespace PgBouncer { - -class ConfigParser { -public: - // Parse a pgbouncer.ini file. Returns true on success. - // On failure, errors are populated in config.errors. - // If resolve_includes is true, %include directives are followed. - // If resolve_referenced_files is true, auth_file and auth_hba_file are parsed. - bool parse(const std::string& filepath, Config& config, - bool resolve_includes = true, - bool resolve_referenced_files = true); - -private: - int include_depth_ = 0; - static const int MAX_INCLUDE_DEPTH = 10; - - bool parse_ini(const std::string& filepath, Config& config, - bool resolve_includes, bool resolve_referenced_files); - - // Section parsers - bool parse_global_key(const std::string& key, const std::string& value, - GlobalSettings& settings, const std::string& file, int line, - std::vector& errors); - bool parse_database_entry(const std::string& name, const std::string& connstr, - Database& db, const std::string& file, int line, - std::vector& errors); - bool parse_user_entry(const std::string& name, const std::string& settings_str, - User& user, const std::string& file, int line, - std::vector& errors); - bool parse_peer_entry(const std::string& name, const std::string& connstr, - Peer& peer, const std::string& file, int line, - std::vector& errors); - - // Connection string parser (key=value pairs used in [databases], [users], [peers]) - static bool parse_connstr_pairs(const std::string& connstr, - std::vector>& pairs, - const std::string& file, int line, - std::vector& errors); - - // String utilities - static std::string trim(const std::string& s); - static std::string unquote(const std::string& s); - static bool parse_bool(const std::string& value, bool& result); - static bool parse_int(const std::string& value, int& result); - static bool parse_uint(const std::string& value, unsigned int& result); -}; - -} // namespace PgBouncer - -#endif diff --git a/lib/pgbouncer_compat/PgBouncer_HBAParser.cpp b/lib/pgbouncer_compat/PgBouncer_HBAParser.cpp index 9b6ec19b81..6e7ff34809 100644 --- a/lib/pgbouncer_compat/PgBouncer_HBAParser.cpp +++ b/lib/pgbouncer_compat/PgBouncer_HBAParser.cpp @@ -41,17 +41,28 @@ static bool looks_like_address(const std::string& tok) { // as single tokens (quotes are stripped from the result). // --------------------------------------------------------------------------- -std::vector HBAParser::tokenize(const std::string& line) { - std::vector tokens; +bool HBAParser::tokenize(const std::string& line, + std::vector& tokens) { + tokens.clear(); std::string token; bool in_quotes = false; + // A token that came from a quoted run is emitted even when empty, so that + // `""` is a real (empty) field rather than silently disappearing. + bool quoted_token = false; for (size_t i = 0; i < line.size(); ++i) { char c = line[i]; if (in_quotes) { if (c == '"') { - in_quotes = false; + // PostgreSQL/PgBouncer escape a quote inside a quoted string by + // doubling it (""). + if (i + 1 < line.size() && line[i + 1] == '"') { + token += '"'; + ++i; + } else { + in_quotes = false; + } } else { token += c; } @@ -61,20 +72,24 @@ std::vector HBAParser::tokenize(const std::string& line) { break; } else if (c == '"') { in_quotes = true; + quoted_token = true; } else if (std::isspace(static_cast(c))) { - if (!token.empty()) { + if (!token.empty() || quoted_token) { tokens.push_back(token); token.clear(); + quoted_token = false; } } else { token += c; } } } - if (!token.empty()) { + if (!token.empty() || quoted_token) { tokens.push_back(token); } - return tokens; + // An unterminated quote means the line is malformed; the caller must not + // treat the partial tokens as a valid record. + return !in_quotes; } // --------------------------------------------------------------------------- @@ -172,6 +187,10 @@ bool HBAParser::parse_record(const std::vector& tokens, bool HBAParser::parse(const std::string& filepath, std::vector& rules, std::vector& errors) { + // parse() is a full load, not an append: reusing the vector across calls + // must not accumulate duplicate rules. `errors` is deliberately left alone, + // since callers thread one diagnostic list through several parsers. + rules.clear(); std::ifstream in(filepath); if (!in.is_open()) { errors.push_back({filepath, 0, @@ -186,7 +205,13 @@ bool HBAParser::parse(const std::string& filepath, while (std::getline(in, line)) { ++lineno; - std::vector tokens = tokenize(line); + std::vector tokens; + if (!tokenize(line, tokens)) { + errors.push_back({filepath, lineno, + "unterminated double quote"}); + ok = false; + continue; + } if (tokens.empty()) continue; diff --git a/lib/pgbouncer_compat/PgBouncer_HBAParser.h b/lib/pgbouncer_compat/PgBouncer_HBAParser.h deleted file mode 100644 index 6d3fc373ed..0000000000 --- a/lib/pgbouncer_compat/PgBouncer_HBAParser.h +++ /dev/null @@ -1,37 +0,0 @@ -#ifndef PGBOUNCER_HBA_PARSER_H -#define PGBOUNCER_HBA_PARSER_H - -#include "PgBouncer_Config.h" -#include -#include - -namespace PgBouncer { - -class HBAParser { -public: - // Parse a pg_hba.conf file as understood by PgBouncer. - // PgBouncer supports a subset of PostgreSQL's HBA format: - // Record types: local, host, hostssl, hostnossl - // Database: all, sameuser, specific name, @file - // User: all, specific name, @file - // Address: IPv4/CIDR, IPv6/CIDR, "all" (for host/hostssl/hostnossl) - // Methods: trust, reject, md5, scram-sha-256, password, cert, peer, ldap, pam - // Options: key=value pairs after the method (e.g., map=mymap) - bool parse(const std::string& filepath, - std::vector& rules, - std::vector& errors); - -private: - // Tokenize a line respecting double-quoted strings - static std::vector tokenize(const std::string& line); - - // Parse a single HBA record from tokens - bool parse_record(const std::vector& tokens, - HBARule& rule, - const std::string& file, int line, - std::vector& errors); -}; - -} // namespace PgBouncer - -#endif diff --git a/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp b/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp index 2e25e65f6f..dd392221dd 100644 --- a/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp +++ b/lib/pgbouncer_compat/PgBouncer_ShowCommands.cpp @@ -68,6 +68,15 @@ static std::vector tokenize_upper(const std::string& s) { // Query generators // --------------------------------------------------------------------------- +// Every generator emits the PgBouncer-compatible columns first, in PgBouncer's +// own order, so existing tooling keeps working. `extended` appends +// ProxySQL-specific columns to the right, per SHOW EXTENDED . +// +// Column names below must match the admin schemas in +// ProxySQL_Admin_Tables_Definitions.h. In particular stats_pgsql_connection_pool +// carries only runtime counters -- weight/max_connections/max_replication_lag +// live in runtime_pgsql_servers, so anything needing them joins that table. + static std::string query_pools(bool extended) { std::string q = "SELECT " @@ -86,8 +95,9 @@ static std::string query_pools(bool extended) { "'statement' AS pool_mode"; if (extended) { q += ", cp.hostgroup AS hostgroup_id" - ", 1 AS multiplex" - ", cp.Latency_us AS latency_us" + ", cp.srv_port AS srv_port" + ", cp.status AS status" + ", cp.Latency_us AS Latency_us" ", cp.Queries AS Queries" ", cp.Bytes_data_sent AS Bytes_data_sent" ", cp.Bytes_data_recv AS Bytes_data_recv"; @@ -96,8 +106,8 @@ static std::string query_pools(bool extended) { return q; } -static std::string query_stats(bool /*extended*/) { - return +static std::string query_stats(bool extended) { + std::string q = "SELECT " "'default' AS database, " "SUM(count_star) AS total_xact_count, " @@ -113,8 +123,16 @@ static std::string query_stats(bool /*extended*/) { "0 AS avg_sent, " "CASE WHEN SUM(count_star) > 0 THEN SUM(sum_time)/SUM(count_star) ELSE 0 END AS avg_xact_time, " "CASE WHEN SUM(count_star) > 0 THEN SUM(sum_time)/SUM(count_star) ELSE 0 END AS avg_query_time, " - "0 AS avg_wait_time " - "FROM stats_pgsql_query_digest"; + "0 AS avg_wait_time"; + if (extended) { + q += ", COUNT(*) AS digest_count" + ", MIN(min_time) AS min_time" + ", MAX(max_time) AS max_time" + ", SUM(sum_rows_sent) AS sum_rows_sent" + ", SUM(sum_rows_affected) AS sum_rows_affected"; + } + q += " FROM stats_pgsql_query_digest"; + return q; } static std::string query_servers(bool extended) { @@ -123,9 +141,9 @@ static std::string query_servers(bool extended) { "'S' AS type, " "'' AS user, " "'' AS database, " - "CASE WHEN ConnUsed > 0 THEN 'active' ELSE 'idle' END AS state, " - "srv_host AS addr, " - "srv_port AS port, " + "CASE WHEN cp.ConnUsed > 0 THEN 'active' ELSE 'idle' END AS state, " + "cp.srv_host AS addr, " + "cp.srv_port AS port, " "'' AS local_addr, " "0 AS local_port, " "'' AS connect_time, " @@ -140,31 +158,39 @@ static std::string query_servers(bool extended) { "'' AS application_name, " "0 AS prepared_statements"; if (extended) { - q += ", hostgroup AS hostgroup" - ", weight AS weight" - ", status AS status" - ", max_replication_lag AS max_replication_lag" - ", Latency_us AS Latency_us" - ", ConnUsed AS ConnUsed" - ", ConnFree AS ConnFree" - ", ConnOK AS ConnOK" - ", ConnERR AS ConnERR"; + q += ", cp.hostgroup AS hostgroup" + ", cp.status AS status" + ", s.weight AS weight" + ", s.max_connections AS max_connections" + ", s.max_replication_lag AS max_replication_lag" + ", s.use_ssl AS use_ssl" + ", cp.Latency_us AS Latency_us" + ", cp.ConnUsed AS ConnUsed" + ", cp.ConnFree AS ConnFree" + ", cp.ConnOK AS ConnOK" + ", cp.ConnERR AS ConnERR"; + } + q += " FROM stats_pgsql_connection_pool cp"; + if (extended) { + q += " LEFT JOIN runtime_pgsql_servers s" + " ON s.hostgroup_id = cp.hostgroup" + " AND s.hostname = cp.srv_host" + " AND s.port = cp.srv_port"; } - q += " FROM stats_pgsql_connection_pool"; return q; } -static std::string query_clients(bool /*extended*/) { - return +static std::string query_clients(bool extended) { + std::string q = "SELECT " "'C' AS type, " "user AS user, " - "db AS database, " + "database AS database, " "CASE WHEN command = 'Sleep' THEN 'idle' ELSE 'active' END AS state, " "cli_host AS addr, " "cli_port AS port, " - "'' AS local_addr, " - "0 AS local_port, " + "l_srv_host AS local_addr, " + "l_srv_port AS local_port, " "time_ms AS connect_time, " "time_ms AS request_time, " "0 AS wait, " @@ -172,19 +198,32 @@ static std::string query_clients(bool /*extended*/) { "0 AS close_needed, " "'' AS ptr, " "'' AS link, " - "0 AS remote_pid, " + "backend_pid AS remote_pid, " "'' AS tls, " "extended_info AS application_name, " - "0 AS prepared_statements " - "FROM stats_pgsql_processlist"; + "0 AS prepared_statements"; + if (extended) { + q += ", ThreadID AS ThreadID" + ", SessionID AS SessionID" + ", hostgroup AS hostgroup" + ", srv_host AS srv_host" + ", srv_port AS srv_port" + ", backend_state AS backend_state" + ", command AS command" + ", info AS info"; + } + q += " FROM stats_pgsql_processlist"; + return q; } -static std::string query_databases(bool /*extended*/) { - return +static std::string query_databases(bool extended) { + // PgBouncer's SHOW DATABASES lists configured databases, so the configured + // server table is the right source -- not the runtime counter table. + std::string q = "SELECT " - "srv_host AS name, " - "srv_host AS host, " - "srv_port AS port, " + "hostname AS name, " + "hostname AS host, " + "port AS port, " "'' AS database, " "'' AS force_user, " "max_connections AS pool_size, " @@ -192,54 +231,94 @@ static std::string query_databases(bool /*extended*/) { "0 AS reserve_pool, " "'statement' AS pool_mode, " "max_connections AS max_connections, " - "ConnUsed + ConnFree AS current_connections, " + "0 AS current_connections, " "0 AS paused, " - "CASE WHEN status = 'ONLINE' THEN 0 ELSE 1 END AS disabled " - "FROM stats_pgsql_connection_pool"; + "CASE WHEN status = 'ONLINE' THEN 0 ELSE 1 END AS disabled"; + if (extended) { + q += ", hostgroup_id AS hostgroup_id" + ", status AS status" + ", weight AS weight" + ", use_ssl AS use_ssl" + ", max_replication_lag AS max_replication_lag" + ", comment AS comment"; + } + q += " FROM runtime_pgsql_servers ORDER BY hostgroup_id, hostname, port"; + return q; } -static std::string query_users(bool /*extended*/) { - return +static std::string query_users(bool extended) { + std::string q = "SELECT " "username AS name, " "CASE WHEN fast_forward=1 THEN 'session' " "WHEN transaction_persistent=1 THEN 'transaction' " - "ELSE 'statement' END AS pool_mode " - "FROM runtime_pgsql_users " - "WHERE active=1 " - "ORDER BY username"; + "ELSE 'statement' END AS pool_mode"; + if (extended) { + q += ", default_hostgroup AS default_hostgroup" + ", max_connections AS max_connections" + ", use_ssl AS use_ssl" + ", transaction_persistent AS transaction_persistent" + ", fast_forward AS fast_forward" + ", backend AS backend" + ", frontend AS frontend"; + } + q += " FROM runtime_pgsql_users WHERE active=1 ORDER BY username"; + return q; } -static std::string query_config(bool /*extended*/) { - return +static std::string query_config(bool extended) { + std::string q = "SELECT " "REPLACE(variable_name, 'pgsql-', '') AS key, " "variable_value AS value, " "'' AS `default`, " - "'yes' AS changeable " - "FROM global_variables " - "WHERE variable_name LIKE 'pgsql-%' " - "ORDER BY variable_name"; + "'yes' AS changeable"; + if (extended) { + q += ", variable_name AS proxysql_variable_name"; + } + q += " FROM global_variables WHERE variable_name LIKE 'pgsql-%' ORDER BY variable_name"; + return q; } -static std::string query_version(bool /*extended*/) { - return +static std::string query_version(bool extended) { + std::string q = "SELECT 'ProxySQL ' || " "(SELECT variable_value FROM global_variables WHERE variable_name='admin-version') " "|| ' (PgBouncer compatibility mode)' AS version"; + if (extended) { + q += ", (SELECT variable_value FROM global_variables " + "WHERE variable_name='admin-version') AS proxysql_version"; + } + return q; } -static std::string query_state(bool /*extended*/) { - return "SELECT 'active' AS state"; +static std::string query_state(bool extended) { + std::string q = "SELECT 'active' AS state"; + if (extended) { + q += ", (SELECT variable_value FROM global_variables " + "WHERE variable_name='admin-version') AS proxysql_version"; + } + return q; } -static std::string query_lists(bool /*extended*/) { - return - "SELECT 'databases' AS list, COUNT(DISTINCT srv_host) AS items FROM stats_pgsql_connection_pool " - "UNION ALL SELECT 'users', COUNT(*) FROM runtime_pgsql_users WHERE active=1 " - "UNION ALL SELECT 'pools', COUNT(*) FROM stats_pgsql_connection_pool " - "UNION ALL SELECT 'free_servers', SUM(ConnFree) FROM stats_pgsql_connection_pool " - "UNION ALL SELECT 'used_servers', SUM(ConnUsed) FROM stats_pgsql_connection_pool"; +static std::string query_lists(bool extended) { + // Each UNION branch must project the same number of columns, so the + // extended form appends its column to every branch. + const char* dbs = extended ? ", 'runtime_pgsql_servers' AS source" : ""; + const char* usr = extended ? ", 'runtime_pgsql_users'" : ""; + const char* pool = extended ? ", 'stats_pgsql_connection_pool'" : ""; + std::string q; + q += std::string("SELECT 'databases' AS list, COUNT(*) AS items") + dbs + + " FROM runtime_pgsql_servers"; + q += std::string(" UNION ALL SELECT 'users', COUNT(*)") + usr + + " FROM runtime_pgsql_users WHERE active=1"; + q += std::string(" UNION ALL SELECT 'pools', COUNT(*)") + pool + + " FROM stats_pgsql_connection_pool"; + q += std::string(" UNION ALL SELECT 'free_servers', COALESCE(SUM(ConnFree),0)") + pool + + " FROM stats_pgsql_connection_pool"; + q += std::string(" UNION ALL SELECT 'used_servers', COALESCE(SUM(ConnUsed),0)") + pool + + " FROM stats_pgsql_connection_pool"; + return q; } // --------------------------------------------------------------------------- @@ -267,7 +346,10 @@ bool translate_show_command(const char* query, int query_len, cmd_idx = 2; } - if (cmd_idx >= tokens.size()) { + // The command word must be the last token: "SHOW POOLS foo" is not a + // PgBouncer command and must fall through to normal admin handling rather + // than being silently treated as "SHOW POOLS". + if (cmd_idx >= tokens.size() || cmd_idx + 1 != tokens.size()) { return false; } diff --git a/lib/pgbouncer_compat/PgBouncer_ShowCommands.h b/lib/pgbouncer_compat/PgBouncer_ShowCommands.h deleted file mode 100644 index f6018f213f..0000000000 --- a/lib/pgbouncer_compat/PgBouncer_ShowCommands.h +++ /dev/null @@ -1,22 +0,0 @@ -#ifndef PGBOUNCER_SHOW_COMMANDS_H -#define PGBOUNCER_SHOW_COMMANDS_H - -#include - -namespace PgBouncer { - -// Checks if a query is a PgBouncer-compatible SHOW command. -// Returns true if the query matches "SHOW [EXTENDED] " -// where is a known PgBouncer command. -// If matched, sets out_query to the equivalent ProxySQL SQL query. -// If not matched, returns false and the caller should handle normally. -bool translate_show_command(const char* query, int query_len, - std::string& out_query, bool& is_extended); - -// Returns an error message for unsupported PgBouncer SHOW commands, -// or empty string if the command is not a known unsupported command. -std::string get_unsupported_show_message(const char* query, int query_len); - -} // namespace PgBouncer - -#endif diff --git a/lib/pgbouncer_compat/ProxySQL_CLI.cpp b/lib/pgbouncer_compat/ProxySQL_CLI.cpp index c31af5abb9..6d75aa29a0 100644 --- a/lib/pgbouncer_compat/ProxySQL_CLI.cpp +++ b/lib/pgbouncer_compat/ProxySQL_CLI.cpp @@ -7,6 +7,18 @@ #include #include +// Single-quote a path for the copy-pasteable example command, so a path with +// spaces or shell metacharacters does not turn into something else when run. +static std::string shell_quote(const std::string& s) { + std::string out = "'"; + for (char c : s) { + if (c == '\'') out += "'\\''"; + else out += c; + } + out += "'"; + return out; +} + static void print_usage() { std::cerr << "Usage: proxysql-cli [options]\n" << "\n" @@ -28,6 +40,16 @@ static int cmd_import_pgbouncer(int argc, const char* argv[]) { return 1; } + // argv[2] is the config path, not an option. Catching an option here turns + // "import-pgbouncer --dry-run" into a usage error rather than a confusing + // "cannot open file: --dry-run". + if (argv[2][0] == '-') { + std::cerr << "Error: import-pgbouncer requires a config file path " + "before any options (got '" << argv[2] << "').\n\n"; + print_usage(); + return 1; + } + std::string config_path = argv[2]; bool dry_run = false; bool ignore_warnings = false; @@ -116,7 +138,7 @@ static int cmd_import_pgbouncer(int argc, const char* argv[]) { << result.rule_count << " query rules, " << result.variable_count << " variables.\n" << "Pipe the output to ProxySQL admin interface to apply:\n" - << " proxysql-cli import-pgbouncer " << config_path + << " proxysql-cli import-pgbouncer " << shell_quote(config_path) << " | mysql -h 127.0.0.1 -P 6032 -u admin -p\n"; return 0; } diff --git a/lib/pgbouncer_compat/ProxySQL_CLI.h b/lib/pgbouncer_compat/ProxySQL_CLI.h deleted file mode 100644 index 8853f453fe..0000000000 --- a/lib/pgbouncer_compat/ProxySQL_CLI.h +++ /dev/null @@ -1,9 +0,0 @@ -#ifndef PROXYSQL_CLI_H -#define PROXYSQL_CLI_H - -// Entry point for proxysql-cli mode. -// Called when argv[0] ends with "proxysql-cli". -// Returns the exit code (0 success, 1 error). -int proxysql_cli_main(int argc, const char* argv[]); - -#endif diff --git a/test/tap/tests/unit/fixtures/pgbouncer_compat/hba_quoting.conf b/test/tap/tests/unit/fixtures/pgbouncer_compat/hba_quoting.conf new file mode 100644 index 0000000000..83c321bfde --- /dev/null +++ b/test/tap/tests/unit/fixtures/pgbouncer_compat/hba_quoting.conf @@ -0,0 +1,4 @@ +# HBA quoting edge cases +host "db""quoted" "user name" 10.0.0.0/8 md5 +host "" all 10.1.0.0/16 trust +host all all 10.2.0.0/16 md5 # trailing comment diff --git a/test/tap/tests/unit/fixtures/pgbouncer_compat/hba_unterminated.conf b/test/tap/tests/unit/fixtures/pgbouncer_compat/hba_unterminated.conf new file mode 100644 index 0000000000..f8d8335bf9 --- /dev/null +++ b/test/tap/tests/unit/fixtures/pgbouncer_compat/hba_unterminated.conf @@ -0,0 +1 @@ +host "unterminated all 10.0.0.0/8 md5 diff --git a/test/tap/tests/unit/fixtures/pgbouncer_compat/quoting.ini b/test/tap/tests/unit/fixtures/pgbouncer_compat/quoting.ini new file mode 100644 index 0000000000..dda8e75910 --- /dev/null +++ b/test/tap/tests/unit/fixtures/pgbouncer_compat/quoting.ini @@ -0,0 +1,15 @@ +; Values exercising quote and inline-comment handling in the [pgbouncer] section. +[pgbouncer] +listen_port = 6432 +auth_type = trust +; a quoted value keeps its spaces and its '#' +auth_query = 'SELECT usename, passwd FROM pg_shadow WHERE usename=$1 # not a comment' +; an unquoted value is truncated at the inline comment +logfile = /var/log/pgbouncer.log # where the log goes +; a quoted value with a doubled quote inside +unix_socket_dir = 'A''B' +; a ';' comment after an unquoted value +pidfile = /var/run/pgbouncer.pid ; the pid file + +[databases] +mydb = host=127.0.0.1 port=5432 diff --git a/test/tap/tests/unit/pgbouncer_config_parser_unit-t.cpp b/test/tap/tests/unit/pgbouncer_config_parser_unit-t.cpp index c682730330..1f80ccfb72 100644 --- a/test/tap/tests/unit/pgbouncer_config_parser_unit-t.cpp +++ b/test/tap/tests/unit/pgbouncer_config_parser_unit-t.cpp @@ -345,11 +345,123 @@ void test_defaults() { CHECK_INT(config.global.max_prepared_statements, 200, "default max_prepared_statements is 200"); } + +// ============================================================ +// Test: quoting and inline comments in the [pgbouncer] section +// +// Regression: comment stripping was skipped entirely for any value starting +// with a quote (so '...' kept its quotes and a '#' inside a quoted value could +// still truncate a value that did not start with one). +// ============================================================ +void test_global_value_quoting() { + PgBouncer::Config config; + bool ok_result = PgBouncer::parse_config_file( + "fixtures/pgbouncer_compat/quoting.ini", config); + + CHECK(ok_result, "quoting.ini parses successfully"); + CHECK(config.errors.empty(), "quoting.ini has no errors"); + + // Quoted value: quotes stripped, '#' inside preserved as data. + CHECK_STR(config.global.auth_query, + "SELECT usename, passwd FROM pg_shadow WHERE usename=$1 # not a comment", + "quoted value keeps its '#' and loses its quotes"); + + // Unquoted value: truncated at the inline '#' comment. + CHECK_STR(config.global.logfile, "/var/log/pgbouncer.log", + "unquoted value is truncated at '#'"); + + // Doubled quote inside a quoted value is a literal quote. + CHECK_STR(config.global.unix_socket_dir, "A'B", + "doubled single quote unescapes to one quote"); + + // ';' also starts an inline comment. + CHECK_STR(config.global.pidfile, "/var/run/pgbouncer.pid", + "unquoted value is truncated at ';'"); +} + +// ============================================================ +// Test: parse() is a full load, not an append +// +// Regression: parsing twice into the same Config duplicated every database, +// user and rule, and kept stale errors alive. +// ============================================================ +void test_reparse_is_not_additive() { + PgBouncer::Config config; + PgBouncer::parse_config_file("fixtures/pgbouncer_compat/full.ini", config); + size_t dbs_first = config.databases.size(); + size_t users_first = config.users.size(); + + CHECK(dbs_first > 0, "first parse found databases"); + + PgBouncer::parse_config_file("fixtures/pgbouncer_compat/full.ini", config); + CHECK(config.databases.size() == dbs_first, + "re-parsing does not duplicate databases"); + CHECK(config.users.size() == users_first, + "re-parsing does not duplicate users"); + + // A failed parse into a previously-populated Config must not leave the + // old content behind. + PgBouncer::parse_config_file("fixtures/pgbouncer_compat/minimal.ini", config); + CHECK(config.databases.size() != dbs_first || dbs_first == 1, + "parsing a different file replaces the previous content"); +} + +// ============================================================ +// Test: HBA tokenizer quoting +// ============================================================ +void test_hba_quoting() { + PgBouncer::Config config; + std::vector rules; + std::vector errors; + + bool ok_result = PgBouncer::parse_hba_file( + "fixtures/pgbouncer_compat/hba_quoting.conf", rules, errors); + + CHECK(ok_result, "hba_quoting.conf parses successfully"); + CHECK_INT((int)rules.size(), 3, "three HBA rules parsed"); + + if (rules.size() >= 2) { + // "db""quoted" -> db"quoted ; "user name" keeps its space + CHECK_STR(rules[0].database, "db\"quoted", + "doubled double-quote unescapes inside an HBA token"); + CHECK_STR(rules[0].user, "user name", + "quoted HBA token keeps its space"); + // "" is an empty token, not a dropped one + CHECK_STR(rules[1].database, "", + "empty quoted HBA token is preserved as an empty field"); + CHECK_STR(rules[1].user, "all", + "field after an empty quoted token is not shifted"); + } else { + CHECK(false, "doubled double-quote unescapes inside an HBA token"); + CHECK(false, "quoted HBA token keeps its space"); + CHECK(false, "empty quoted HBA token is preserved as an empty field"); + CHECK(false, "field after an empty quoted token is not shifted"); + } +} + +// ============================================================ +// Test: an unterminated quote in pg_hba.conf is an error +// +// Regression: tokenize() returned partial tokens for a line with a dangling +// quote, so the record was accepted with silently truncated fields. +// ============================================================ +void test_hba_unterminated_quote() { + std::vector rules; + std::vector errors; + + bool ok_result = PgBouncer::parse_hba_file( + "fixtures/pgbouncer_compat/hba_unterminated.conf", rules, errors); + + CHECK(!ok_result, "unterminated quote fails the HBA parse"); + CHECK(!errors.empty(), "unterminated quote reports an error"); + CHECK(rules.empty(), "no rule is produced from the malformed line"); +} + // ============================================================ // Main // ============================================================ int main() { - plan(127); + plan(146); test_minimal_config(); // 7 tests test_full_config(); // 42 tests @@ -359,6 +471,10 @@ int main() { test_include_directive(); // 7 tests test_nonexistent_file(); // 2 tests test_defaults(); // 9 tests (adjusted: removed 1 duplicate) + test_global_value_quoting(); + test_reparse_is_not_additive(); + test_hba_quoting(); + test_hba_unterminated_quote(); return exit_status(); } diff --git a/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp b/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp index 4e6e6c3dc3..3ea7ff7f83 100644 --- a/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp +++ b/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp @@ -274,8 +274,175 @@ void test_tls_conversion() { CHECK(has_sql_containing(result, "/etc/ssl/ca.pem"), "correct CA path"); } + +// Helper: does any warning or error mention this substring? +static bool has_issue_containing(const PgBouncer::ConversionResult& r, + const std::string& substr) { + for (const auto& w : r.warnings) + if (w.message.find(substr) != std::string::npos) return true; + for (const auto& e : r.errors) + if (e.message.find(substr) != std::string::npos) return true; + return false; +} + +// ============================================================ +// Test: generated SQL uses the real pgsql_* column names +// +// Regression: routing rules were emitted as +// INSERT INTO pgsql_query_rules (..., schemaname, ...) +// but pgsql_query_rules has no `schemaname` column (that is the MySQL table); +// the PgSQL one calls it `database`. Every generated rule failed on execute. +// ============================================================ +void test_query_rule_column_names() { + PgBouncer::Config config; + PgBouncer::Database db; + db.name = "mydb"; + db.host = "10.0.0.1"; + config.databases.push_back(db); + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + CHECK(!has_sql_containing(result, "schemaname"), + "query rules do not reference the nonexistent `schemaname` column"); + CHECK(has_sql_containing(result, "(rule_id, active, database, destination_hostgroup, apply)"), + "query rules use the `database` column"); +} + +// ============================================================ +// Test: firewall whitelist INSERT satisfies the table's NOT NULL columns +// +// pgsql_firewall_whitelist_rules declares `digest` and `comment` NOT NULL with +// no default, and names the database column `database`. +// ============================================================ +void test_firewall_rule_columns() { + PgBouncer::Config config; + PgBouncer::Database db; + db.name = "mydb"; + db.host = "10.0.0.1"; + config.databases.push_back(db); + + PgBouncer::HBARule rule; + rule.conn_type = "host"; + rule.database = "all"; + rule.user = "all"; + rule.address = "10.0.0.0/8"; + rule.method = "md5"; + config.hba_rules.push_back(rule); + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + CHECK(has_sql_containing(result, + "(active, client_address, username, database, flagIN, digest, comment)"), + "firewall INSERT lists database, digest and comment"); + CHECK(!has_sql_containing(result, "pgsql_firewall_whitelist_rules (active, client_address, username, schemaname"), + "firewall INSERT does not use `schemaname`"); +} + +// ============================================================ +// Test: HBA reject rules are reported, not silently swallowed +// +// The ProxySQL whitelist is allow-only, so a `reject` cannot be reproduced. +// It used to be emitted as an SQL comment and still flipped the whitelist on. +// ============================================================ +void test_hba_reject_is_reported() { + PgBouncer::Config config; + PgBouncer::Database db; + db.name = "mydb"; + db.host = "10.0.0.1"; + config.databases.push_back(db); + + PgBouncer::HBARule rule; + rule.conn_type = "host"; + rule.database = "all"; + rule.user = "baduser"; + rule.address = "192.168.0.0/16"; + rule.method = "reject"; + config.hba_rules.push_back(rule); + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult relaxed = converter.convert(config, false); + CHECK(has_issue_containing(relaxed, "reject"), + "reject rule raises an issue in relaxed mode"); + + PgBouncer::ConfigConverter strict_conv; + PgBouncer::ConversionResult strict = strict_conv.convert(config, true); + CHECK(!strict.success, "reject rule fails the import in strict mode"); +} + +// ============================================================ +// Test: hashed auth-file passwords are flagged +// +// ProxySQL derives the MD5 challenge response and the SCRAM verifier from the +// cleartext password in pgsql_users.password, so importing a pre-hashed +// userlist.txt entry produces a credential that cannot authenticate. +// ============================================================ +void test_hashed_password_is_flagged() { + PgBouncer::Config config; + PgBouncer::Database db; + db.name = "mydb"; + db.host = "10.0.0.1"; + config.databases.push_back(db); + + PgBouncer::AuthFileEntry md5e; + md5e.username = "alice"; + md5e.password = "md5d41d8cd98f00b204e9800998ecf8427"; + md5e.type = PgBouncer::AuthType::MD5; + config.auth_entries.push_back(md5e); + + PgBouncer::AuthFileEntry scram; + scram.username = "bob"; + scram.password = "SCRAM-SHA-256$4096:c2FsdA==$c3Ry:c3Ry"; + scram.type = PgBouncer::AuthType::SCRAM; + config.auth_entries.push_back(scram); + + PgBouncer::AuthFileEntry plain; + plain.username = "carol"; + plain.password = "secret"; + plain.type = PgBouncer::AuthType::PLAIN; + config.auth_entries.push_back(plain); + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + CHECK(has_issue_containing(result, "alice"), "MD5 verifier for alice is flagged"); + CHECK(has_issue_containing(result, "bob"), "SCRAM verifier for bob is flagged"); + CHECK(!has_issue_containing(result, "carol"), "cleartext password for carol is not flagged"); + CHECK(has_sql_containing(result, "'carol'"), "carol is still imported"); + + PgBouncer::ConfigConverter strict_conv; + PgBouncer::ConversionResult strict = strict_conv.convert(config, true); + CHECK(!strict.success, "hashed passwords fail the import in strict mode"); +} + +// ============================================================ +// Test: a dbname alias is reported rather than silently dropped +// +// PgBouncer's `dbname=` connects to a differently-named backend database. +// ProxySQL routes to a hostgroup but does not rewrite the startup packet. +// ============================================================ +void test_dbname_alias_is_reported() { + PgBouncer::Config config; + PgBouncer::Database db; + db.name = "alias"; + db.dbname = "real_backend_db"; + db.host = "10.0.0.1"; + config.databases.push_back(db); + + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + CHECK(has_issue_containing(result, "real_backend_db"), + "dbname alias is reported"); + + PgBouncer::ConfigConverter strict_conv; + PgBouncer::ConversionResult strict = strict_conv.convert(config, true); + CHECK(!strict.success, "dbname alias fails the import in strict mode"); +} + int main() { - plan(39); + plan(52); test_minimal_conversion(); // 6 test_multi_host_conversion(); // 5 @@ -287,7 +454,11 @@ int main() { test_query_rules(); // 5 test_dry_run_format(); // 5 test_tls_conversion(); // 3 + test_query_rule_column_names(); + test_firewall_rule_columns(); + test_hba_reject_is_reported(); + test_hashed_password_is_flagged(); + test_dbname_alias_is_reported(); - // Note: plan count = sum above. Adjust if needed. return exit_status(); } diff --git a/test/tap/tests/unit/pgbouncer_show_commands_unit-t.cpp b/test/tap/tests/unit/pgbouncer_show_commands_unit-t.cpp index 176ae23e39..1ea0dc5bbe 100644 --- a/test/tap/tests/unit/pgbouncer_show_commands_unit-t.cpp +++ b/test/tap/tests/unit/pgbouncer_show_commands_unit-t.cpp @@ -187,18 +187,98 @@ void test_unsupported_commands() { CHECK(msg.empty(), "SHOW POOLS is not unsupported"); } + +// ============================================================ +// Test: SHOW EXTENDED adds ProxySQL columns for every command +// +// Regression: only POOLS and SERVERS honoured `extended`; the other eight +// generators ignored the flag, so SHOW EXTENDED returned the plain +// PgBouncer column set. +// ============================================================ +static bool extended_differs_from_plain(const char* cmd) { + std::string plain_q, ext_q; + bool e1 = false, e2 = false; + std::string plain = std::string("SHOW ") + cmd; + std::string ext = std::string("SHOW EXTENDED ") + cmd; + if (!PgBouncer::translate_show_command(plain.c_str(), (int)plain.size(), plain_q, e1)) + return false; + if (!PgBouncer::translate_show_command(ext.c_str(), (int)ext.size(), ext_q, e2)) + return false; + return e2 && !e1 && ext_q != plain_q && ext_q.size() > plain_q.size(); +} + +void test_extended_all_commands() { + const char* cmds[] = {"POOLS", "STATS", "SERVERS", "CLIENTS", "DATABASES", + "USERS", "CONFIG", "VERSION", "STATE", "LISTS"}; + for (const char* c : cmds) { + std::string msg = std::string("SHOW EXTENDED ") + c + " adds columns"; + CHECK(extended_differs_from_plain(c), msg.c_str()); + } +} + +// ============================================================ +// Test: translations only reference columns that actually exist +// +// Regression: SHOW CLIENTS selected `db` (the column is `database`), and +// SHOW DATABASES / SHOW EXTENDED SERVERS selected weight, max_connections and +// max_replication_lag from stats_pgsql_connection_pool, which has none of +// them. These are string-level guards; pgbouncer_show_commands-t executes the +// same queries against a live admin interface. +// ============================================================ +void test_no_phantom_columns() { + std::string q; + bool ext = false; + + PgBouncer::translate_show_command("SHOW CLIENTS", 12, q, ext); + CHECK(q.find("db AS database") == std::string::npos, + "SHOW CLIENTS does not select the nonexistent `db` column"); + CHECK(q.find("database AS database") != std::string::npos, + "SHOW CLIENTS selects `database`"); + + PgBouncer::translate_show_command("SHOW DATABASES", 14, q, ext); + CHECK(q.find("stats_pgsql_connection_pool") == std::string::npos, + "SHOW DATABASES no longer reads max_connections from the stats table"); + CHECK(q.find("runtime_pgsql_servers") != std::string::npos, + "SHOW DATABASES reads runtime_pgsql_servers"); + + PgBouncer::translate_show_command("SHOW EXTENDED SERVERS", 21, q, ext); + CHECK(q.find("runtime_pgsql_servers") != std::string::npos, + "SHOW EXTENDED SERVERS joins runtime_pgsql_servers for weight/max_connections"); + CHECK(q.find("s.weight") != std::string::npos, + "SHOW EXTENDED SERVERS takes weight from the servers table"); +} + +// ============================================================ +// Test: trailing tokens are not a PgBouncer command +// +// "SHOW POOLS foo" used to translate as "SHOW POOLS", swallowing a query the +// admin interface should have handled (or rejected) itself. +// ============================================================ +void test_trailing_tokens_rejected() { + std::string q; + bool ext = false; + + CHECK(!PgBouncer::translate_show_command("SHOW POOLS foo", 14, q, ext), + "SHOW POOLS foo is not translated"); + CHECK(!PgBouncer::translate_show_command("SHOW EXTENDED SERVERS bar", 25, q, ext), + "SHOW EXTENDED SERVERS bar is not translated"); + CHECK(PgBouncer::translate_show_command("SHOW POOLS", 10, q, ext), + "SHOW POOLS alone still translates"); +} + int main() { - plan(39); - - test_show_command_recognition(); // 10 - test_case_insensitive(); // 3 - test_trailing_semicolon(); // 3 - test_extended_variant(); // 4 - test_sql_output_columns(); // 10 - test_non_matching_queries(); // 3 - test_unsupported_commands(); // 5 - - // Note: 10+3+3+4+10+3+5 = 38, but plan says 40. - // If count is off, adjust the plan number. + plan(58); + + test_show_command_recognition(); + test_case_insensitive(); + test_trailing_semicolon(); + test_extended_variant(); + test_sql_output_columns(); + test_non_matching_queries(); + test_unsupported_commands(); + test_extended_all_commands(); + test_no_phantom_columns(); + test_trailing_tokens_rejected(); + return exit_status(); } From 462122f71dff38229eef02ca952171061d03f072 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Thu, 27 Aug 2026 06:19:27 +0000 Subject: [PATCH 08/12] test/pkg/docs: run the PgBouncer tests in CI, package proxysql-cli, document it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests were never registered in test/tap/groups/groups.json, so none of them ran in any TAP group. The three unit tests now sit in unit-tests-g1 alongside the other unit tests, and a new integration test joins legacy-g4 next to the other pgsql-* admin tests. The new integration test, pgsql-pgbouncer_compat-t, executes every PgBouncer SHOW command (plain and EXTENDED) and every statement the converter emits against a live PgSQL admin port. That is the coverage that was missing: the unit tests compare generated strings, which is why three statements that could not execute at all passed the whole suite. It also checks that a non-PgBouncer SHOW still reaches ProxySQL's own handling, and that a failed IMPORT leaves the connection usable. CI-pgbouncer-compat.yml is removed rather than fixed. It was pinned to `branches: [feature/pgbouncer-compat]`, so it would have stopped running the moment it merged, and it was a self-contained workflow in a repo where every CI-*.yml on v3.0 is a thin caller into ci-*.yml@GH-Actions (see doc/GH-Actions/README.md). Registering the tests in groups.json is what actually gets them running: legacy-g4 covers the integration test today, and the unit tests are compiled by CI-builds and will run under CI-unittests when that workflow is re-enabled (it is disabled repo-wide, see #5603). Packaging never shipped proxysql-cli, so the argv[0] dispatch in main() was unreachable from an installed package: - rpm (rhel + suse): a real symlink created in %install, picked up by the existing %{_bindir}/* glob so rpm owns and removes it. - deb: created in postinst and removed in postrm, since equivs `Files:` handles regular files only. - tarball: bin/proxysql-cli wrapper execing libexec/proxysql-cli. The extra indirection is needed because the existing bin/proxysql wrapper execs proxysql.bin, which would make argv[0] "proxysql.bin", and `exec -a` is not available in POSIX sh. - make install / uninstall. verify-package-install.bash now checks proxysql-cli is on PATH and dispatches to CLI mode, so a packaging regression fails the release verification instead of shipping. Its self-test gains a matching stub. doc/PGBOUNCER_COMPAT.md replaces the design spec the PR and all three issues pointed at, which lived in a working directory and was never committed. It documents the implemented behaviour: the full parameter mapping, the SHOW support matrix, and — deliberately explicit — everything that is not mapped and why, including the pre-hashed-password limitation. --- .github/workflows/CI-pgbouncer-compat.yml | 50 ---- Makefile | 4 + doc/PGBOUNCER_COMPAT.md | 221 ++++++++++++++ .../proxysql/deb-compliant/ctl/proxysql.ctl | 14 + .../rpmmacros/rpmbuild/SPECS/proxysql.spec | 5 + .../rpmmacros/rpmbuild/SPECS/proxysql.spec | 5 + .../entrypoint/entrypoint.bash | 17 ++ .../control/test-verify-package-install.bash | 8 + .../infra/control/verify-package-install.bash | 22 +- test/tap/groups/groups.json | 4 + test/tap/tests/pgsql-pgbouncer_compat-t.cpp | 275 ++++++++++++++++++ 11 files changed, 574 insertions(+), 51 deletions(-) delete mode 100644 .github/workflows/CI-pgbouncer-compat.yml create mode 100644 doc/PGBOUNCER_COMPAT.md create mode 100644 test/tap/tests/pgsql-pgbouncer_compat-t.cpp diff --git a/.github/workflows/CI-pgbouncer-compat.yml b/.github/workflows/CI-pgbouncer-compat.yml deleted file mode 100644 index 76f132bef7..0000000000 --- a/.github/workflows/CI-pgbouncer-compat.yml +++ /dev/null @@ -1,50 +0,0 @@ -name: CI-pgbouncer-compat - -on: - push: - branches: [ 'feature/pgbouncer-compat' ] - paths: - - 'lib/pgbouncer_compat/**' - - 'include/PgBouncer_*.h' - - 'include/ProxySQL_CLI.h' - - 'test/tap/tests/unit/pgbouncer_*' - - '.github/workflows/CI-pgbouncer-compat.yml' - pull_request: - paths: - - 'lib/pgbouncer_compat/**' - - 'include/PgBouncer_*.h' - - 'include/ProxySQL_CLI.h' - - 'test/tap/tests/unit/pgbouncer_*' - -jobs: - unit-tests: - name: PgBouncer Compat Unit Tests - runs-on: ubuntu-latest - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Install dependencies - run: | - sudo apt-get update -qq - sudo apt-get install -y -qq build-essential g++ make libssl-dev - - - name: Build unit tests - working-directory: test/tap/tests/unit - run: | - make pgbouncer_config_parser_unit-t - make pgbouncer_converter_unit-t - make pgbouncer_show_commands_unit-t - - - name: Run parser tests (127 tests) - working-directory: test/tap/tests/unit - run: ./pgbouncer_config_parser_unit-t - - - name: Run converter tests (39 tests) - working-directory: test/tap/tests/unit - run: ./pgbouncer_converter_unit-t - - - name: Run SHOW commands tests (39 tests) - working-directory: test/tap/tests/unit - run: ./pgbouncer_show_commands_unit-t diff --git a/Makefile b/Makefile index 235f62bfec..2ff5b16e5e 100644 --- a/Makefile +++ b/Makefile @@ -588,6 +588,9 @@ cleanbuild: .PHONY: install install: src/proxysql install -m 0755 src/proxysql /usr/bin + # proxysql-cli is the same binary; main() dispatches on argv[0] and never + # starts the daemon when invoked under this name. + ln -sf proxysql /usr/bin/proxysql-cli install -m 0600 etc/proxysql.cnf /etc if [ ! -d /var/lib/proxysql ]; then mkdir /var/lib/proxysql ; fi if [ -f plugins/mysqlx/ProxySQL_MySQLX_Plugin.so ]; then \ @@ -635,6 +638,7 @@ endif .PHONY: uninstall uninstall: if [ -f /etc/proxysql.cnf ]; then rm /etc/proxysql.cnf ; fi + if [ -L /usr/bin/proxysql-cli ]; then rm /usr/bin/proxysql-cli ; fi if [ -f /usr/bin/proxysql ]; then rm /usr/bin/proxysql ; fi if [ -f /usr/lib/proxysql/plugins/ProxySQL_MySQLX_Plugin.so ]; then rm /usr/lib/proxysql/plugins/ProxySQL_MySQLX_Plugin.so ; fi if [ -f /usr/lib/proxysql/plugins/ProxySQL_GenAI_Plugin.so ]; then rm /usr/lib/proxysql/plugins/ProxySQL_GenAI_Plugin.so ; fi diff --git a/doc/PGBOUNCER_COMPAT.md b/doc/PGBOUNCER_COMPAT.md new file mode 100644 index 0000000000..7bd863550a --- /dev/null +++ b/doc/PGBOUNCER_COMPAT.md @@ -0,0 +1,221 @@ +# PgBouncer compatibility + +ProxySQL can read an existing PgBouncer deployment's configuration and can answer +PgBouncer's `SHOW` commands on its PostgreSQL admin port. Together these let a +PgBouncer installation be replaced without rewriting its configuration by hand and +without changing the monitoring that reads from it. + +The implementation lives in `lib/pgbouncer_compat/` and is built into +`libproxysql.a` for every tier — there is no feature flag. + +> This document describes the behaviour that is actually implemented. Where a +> PgBouncer feature has no ProxySQL equivalent it is listed as such rather than +> approximated, because a silent approximation is worse than a reported gap. + +--- + +## 1. Importing a PgBouncer configuration + +Two entry points, one conversion engine. + +### `proxysql-cli` (offline) + +`proxysql-cli` is the `proxysql` binary under a second name. `main()` compares +`basename(argv[0])` against `proxysql-cli` and dispatches to the CLI without ever +starting the daemon. Packages install it as a symlink next to `proxysql`. + +```bash +# Print the SQL that would be applied, with an explanatory comment per statement +proxysql-cli import-pgbouncer /etc/pgbouncer/pgbouncer.ini --dry-run + +# Warn on unmappable parameters instead of failing +proxysql-cli import-pgbouncer /etc/pgbouncer/pgbouncer.ini --ignore-warnings + +# Apply against a running instance +proxysql-cli import-pgbouncer /etc/pgbouncer/pgbouncer.ini \ + | psql -h 127.0.0.1 -p 6132 -U admin +``` + +### `IMPORT PGBOUNCER CONFIG` (online) + +From the admin interface: + +```sql +IMPORT PGBOUNCER CONFIG FROM '/etc/pgbouncer/pgbouncer.ini'; +IMPORT PGBOUNCER CONFIG FROM '/etc/pgbouncer/pgbouncer.ini' DRY RUN; +IMPORT PGBOUNCER CONFIG FROM '/etc/pgbouncer/pgbouncer.ini' IGNORE WARNINGS; +IMPORT PGBOUNCER CONFIG FROM '/etc/pgbouncer/pgbouncer.ini' DRY RUN IGNORE WARNINGS; +``` + +The path is read by the **server**, not the client. + +### Import-once, strict by default + +The import bootstraps ProxySQL; from then on the admin interface owns the +configuration. There is no continuous sync back to `pgbouncer.ini`. + +By default the import is **strict**: any parameter that cannot be mapped is an +error and nothing is applied. `IGNORE WARNINGS` / `--ignore-warnings` downgrades +those to warnings and applies the rest. Strictness is deliberate — a partially +imported pooler configuration is a production incident waiting to happen. + +The converter rewrites `pgsql_servers`, `pgsql_users` and `pgsql_query_rules` +from scratch (each is `DELETE`d before its `INSERT`s), so an import replaces the +PostgreSQL-side configuration rather than merging into it. + +--- + +## 2. What the parser reads + +| File | Parsed by | Notes | +|---|---|---| +| `pgbouncer.ini` | `PgBouncer_ConfigParser` | `[pgbouncer]`, `[databases]`, `[users]`, `[peers]`; `%include` to 10 levels | +| `userlist.txt` | `PgBouncer_AuthFileParser` | `"user" "password"`; detects plain / MD5 / SCRAM | +| `pg_hba.conf` | `PgBouncer_HBAParser` | followed when `auth_hba_file` is set | + +Quoting follows PgBouncer: a single-quoted value in `[pgbouncer]` protects spaces +and `#`/`;`, and `''` is a literal quote. In `pg_hba.conf`, `""` inside a +double-quoted token is a literal quote, and an unterminated quote is an error +rather than a silently truncated field. + +--- + +## 3. Configuration mapping + +### Global settings + +| PgBouncer | ProxySQL | Note | +|---|---|---| +| `listen_addr` + `listen_port` | `pgsql-interfaces` | | +| `max_client_conn` | `pgsql-max_connections` | | +| `server_connect_timeout` | `pgsql-connect_timeout_server` | | +| `server_lifetime` | `pgsql-connection_max_age_ms` | s → ms | +| `client_idle_timeout` | `pgsql-wait_timeout` | s → ms | +| `log_min_duration` | `pgsql-long_query_time` | | +| `idle_transaction_timeout` | `pgsql-max_transaction_idle_time` | s → ms | +| `transaction_timeout` | `pgsql-max_transaction_time` | s → ms | +| `max_prepared_statements` | `pgsql-max_stmts_per_connection` | | +| `server_tls_sslmode` | `use_ssl` on all imported servers | `require`/`verify-ca`/`verify-full` | +| `server_tls_ca_file` | `pgsql-ssl_p2s_ca` | | +| `server_tls_cert_file` | `pgsql-ssl_p2s_cert` | | +| `server_tls_key_file` | `pgsql-ssl_p2s_key` | | +| `server_check_query` | `pgsql-monitor_enabled=1` | | +| `server_check_delay` | `pgsql-monitor_ping_interval` | s → ms | +| `tcp_keepalive` | `pgsql-use_tcp_keepalive` | | +| `tcp_keepidle` | `pgsql-tcp_keepalive_time` | | + +### `[databases]` + +Each entry becomes a hostgroup with one `pgsql_servers` row per host (a +comma-separated `host=` list yields several rows in the same hostgroup), plus a +`pgsql_query_rules` row routing that database name to the hostgroup. The `*` +wildcard database becomes the default hostgroup and gets no routing rule. +`pool_size` maps to `max_connections` on the server row. + +### `[users]` and `userlist.txt` + +Each user becomes a `pgsql_users` row. `pool_mode` maps as: + +| PgBouncer `pool_mode` | ProxySQL | +|---|---| +| `session` | `fast_forward=1` | +| `transaction` | `transaction_persistent=1` | +| `statement` | neither flag | + +### `pg_hba.conf` + +`host`/`hostssl` records with `trust`, `md5`, `scram-sha-256` or `password` +become `pgsql_firewall_whitelist_rules` entries and enable +`pgsql-firewall_whitelist_enabled`. `hostssl` additionally sets `use_ssl=1` on +the matching users. + +--- + +## 4. What is *not* mapped + +These are reported per occurrence — fatal in strict mode, warnings otherwise. + +**Authentication.** `auth_query`, `auth_user`, `auth_dbname`. ProxySQL +authenticates from `pgsql_users` (or LDAP), not by querying the backend. + +**Pre-hashed passwords.** A `userlist.txt` entry holding an MD5 or SCRAM verifier +is imported verbatim but **will not authenticate**. ProxySQL derives both the MD5 +challenge response and the SCRAM verifier from the *cleartext* password stored in +`pgsql_users.password`, so a pre-hashed value cannot be used. Replace those +entries with the cleartext password after importing. + +**`dbname=` aliases.** PgBouncer's `dbname=` connects to a backend database under +a different name than the client asked for. ProxySQL routes to a hostgroup but +does not rewrite the database in the startup packet, so the alias cannot be +honoured. + +**HBA `reject` rules.** The ProxySQL firewall whitelist is allow-only and has no +deny entry, so a `reject` that precedes a broader allow cannot be reproduced — +the allow would win. Enforce the denial upstream. + +**HBA `local` / `hostnossl` records**, and the `cert`, `peer`, `pam`, `ident`, +`gss` and `sspi` methods. + +**Clustering.** `peer_id` and the `[peers]` section — use ProxySQL Cluster. + +**No equivalent.** `so_reuseport`, `disable_pqexec`, `application_name_add_host`, +`dns_zone_check_period`, `resolv_conf`, a non-default `server_reset_query`, +`sbuf_loopcnt`, `pkt_buf`. + +--- + +## 5. PgBouncer `SHOW` commands + +Available on the **PostgreSQL admin port**. Each command also accepts a +`SHOW EXTENDED ` form that appends ProxySQL-specific columns to the +right of the PgBouncer-compatible ones, so a tool reading by column position or +by the documented PgBouncer names keeps working. + +| Command | Backed by | +|---|---| +| `SHOW POOLS` | `stats_pgsql_connection_pool` | +| `SHOW STATS` | `stats_pgsql_query_digest` | +| `SHOW SERVERS` | `stats_pgsql_connection_pool` (+ `runtime_pgsql_servers` when extended) | +| `SHOW CLIENTS` | `stats_pgsql_processlist` | +| `SHOW DATABASES` | `runtime_pgsql_servers` | +| `SHOW USERS` | `runtime_pgsql_users` | +| `SHOW CONFIG` | `global_variables` (`pgsql-` prefix stripped) | +| `SHOW VERSION` | identifies as ProxySQL in PgBouncer compatibility mode | +| `SHOW STATE` | always `active` | +| `SHOW LISTS` | object counts | + +Rejected with an explanatory error: `SHOW DNS_HOSTS`, `SHOW DNS_ZONES`, +`SHOW FDS`, `SHOW PEERS`, `SHOW PEER_POOLS`, `SHOW MEM`, `SHOW ACTIVE_SOCKETS`, +`SHOW SOCKETS`. + +Some columns are structurally absent from ProxySQL and are reported as `0` or the +empty string rather than omitted, so the column count stays stable: per-client +wait times, `local_addr`/`local_port` on servers, socket pointers (`ptr`, `link`) +and `prepared_statements`. + +### Interaction with ProxySQL's own `SHOW` + +The translation runs **before** ProxySQL's generic `SHOW` handling, because +ProxySQL already owns some of the same command words — `SHOW DATABASES` most +notably. Only the exact command set above is claimed: anything with a trailing +token (`SHOW POOLS foo`) falls through to normal admin handling, as does every +other `SHOW`. + +--- + +## 6. Tests + +| Test | Covers | +|---|---| +| `test/tap/tests/unit/pgbouncer_config_parser_unit-t` | INI / userlist / HBA parsing, quoting, includes, malformed input | +| `test/tap/tests/unit/pgbouncer_converter_unit-t` | mapping rules, strict vs relaxed, generated column names | +| `test/tap/tests/unit/pgbouncer_show_commands_unit-t` | `SHOW` translation and rejection | +| `test/tap/tests/pgsql-pgbouncer_compat-t` | **executes** every `SHOW` and every generated statement against a live admin port | + +The integration test matters disproportionately here. The unit tests compare +generated strings, which cannot catch a wrong column name — and three such +defects (`pgsql_query_rules.schemaname`, `stats_pgsql_processlist.db`, and +reading `weight`/`max_connections` from `stats_pgsql_connection_pool`) passed the +full unit suite while being unable to execute. Any new mapping or `SHOW` +translation must be exercised by `pgsql-pgbouncer_compat-t`, not by a string +comparison alone. diff --git a/docker/images/proxysql/deb-compliant/ctl/proxysql.ctl b/docker/images/proxysql/deb-compliant/ctl/proxysql.ctl index 970431f2ae..840196ceea 100644 --- a/docker/images/proxysql/deb-compliant/ctl/proxysql.ctl +++ b/docker/images/proxysql/deb-compliant/ctl/proxysql.ctl @@ -25,6 +25,10 @@ Description: High performance MySQL and PostgreSQL proxy It features query routing, query caching, query rewriting (for queries generated by ORMs, for example) and is most of the time a drop-in replacement for mysqld and postgres from the point of view of the application. It can be configured and remote controlled through an SQL-compatible admin interface. File: postinst #!/bin/sh -e + # proxysql-cli is the same binary under a second name; main() dispatches on + # argv[0] (see src/main.cpp) and never starts the daemon in that mode. equivs + # `Files:` copies regular files only, so the link is made here. + ln -sf proxysql /usr/bin/proxysql-cli if [ ! -d /var/lib/proxysql ]; then mkdir /var/lib/proxysql ; fi if ! id -u proxysql > /dev/null 2>&1; then useradd -r -U -s /bin/false -d /var/lib/proxysql -c "ProxySQL Server" proxysql; fi chown -R proxysql: /var/lib/proxysql @@ -34,3 +38,13 @@ File: postinst systemctl enable proxysql.service > /dev/null || true systemctl --system daemon-reload > /dev/null || true fi +File: postrm + #!/bin/sh -e + # Remove the proxysql-cli symlink created by postinst. dpkg does not track it + # (equivs `Files:` handles regular files only), so it has to be cleaned up + # explicitly or it would be left dangling after the binary is removed. + case "$1" in + remove|purge) + if [ -L /usr/bin/proxysql-cli ]; then rm -f /usr/bin/proxysql-cli ; fi + ;; + esac diff --git a/docker/images/proxysql/rhel-compliant/rpmmacros/rpmbuild/SPECS/proxysql.spec b/docker/images/proxysql/rhel-compliant/rpmmacros/rpmbuild/SPECS/proxysql.spec index c80629d43e..b9cd1cf1f9 100644 --- a/docker/images/proxysql/rhel-compliant/rpmmacros/rpmbuild/SPECS/proxysql.spec +++ b/docker/images/proxysql/rhel-compliant/rpmmacros/rpmbuild/SPECS/proxysql.spec @@ -41,6 +41,11 @@ mkdir -p %{buildroot} cp -a * %{buildroot} mkdir -p %{buildroot}/var/run/%{name} mkdir -p %{buildroot}/var/lib/%{name} +# proxysql-cli is the same binary under a second name; main() dispatches on +# argv[0] (see src/main.cpp) and never starts the daemon in that mode. Shipped +# as a real symlink so rpm owns it and removes it on uninstall; the existing +# %{_bindir}/* glob in %files already picks it up. +ln -sf proxysql %{buildroot}%{_bindir}/proxysql-cli %clean rm -rf %{buildroot} diff --git a/docker/images/proxysql/suse-compliant/rpmmacros/rpmbuild/SPECS/proxysql.spec b/docker/images/proxysql/suse-compliant/rpmmacros/rpmbuild/SPECS/proxysql.spec index f36166b6d8..78268c22c2 100644 --- a/docker/images/proxysql/suse-compliant/rpmmacros/rpmbuild/SPECS/proxysql.spec +++ b/docker/images/proxysql/suse-compliant/rpmmacros/rpmbuild/SPECS/proxysql.spec @@ -41,6 +41,11 @@ mkdir -p %{buildroot} cp -a * %{buildroot} mkdir -p %{buildroot}/var/run/%{name} mkdir -p %{buildroot}/var/lib/%{name} +# proxysql-cli is the same binary under a second name; main() dispatches on +# argv[0] (see src/main.cpp) and never starts the daemon in that mode. Shipped +# as a real symlink so rpm owns it and removes it on uninstall; the existing +# %{_bindir}/* glob in %files already picks it up. +ln -sf proxysql %{buildroot}%{_bindir}/proxysql-cli %clean rm -rf %{buildroot} diff --git a/docker/images/proxysql/tarball-compliant/entrypoint/entrypoint.bash b/docker/images/proxysql/tarball-compliant/entrypoint/entrypoint.bash index e66f03fdc8..3e80c2d0d7 100755 --- a/docker/images/proxysql/tarball-compliant/entrypoint/entrypoint.bash +++ b/docker/images/proxysql/tarball-compliant/entrypoint/entrypoint.bash @@ -53,6 +53,23 @@ exec "${BIN_DIR}/proxysql.bin" "$@" EOF chmod 0755 "pkgroot/${DIR_NAME}/bin/proxysql" +# proxysql-cli: the same binary, selected by argv[0]. main() compares the +# basename against "proxysql-cli" exactly, and the wrapper above execs +# proxysql.bin -- which would make argv[0] "proxysql.bin". So the wrapper for +# the CLI execs a correctly-named symlink under libexec/ instead. `exec -a` is +# not available in POSIX sh, which is why this needs the extra indirection. +mkdir -p "pkgroot/${DIR_NAME}/libexec" +ln -sf ../bin/proxysql.bin "pkgroot/${DIR_NAME}/libexec/proxysql-cli" +cat > "pkgroot/${DIR_NAME}/bin/proxysql-cli" <<'EOF' +#!/bin/sh +set -eu + +BIN_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) +export LD_LIBRARY_PATH="${BIN_DIR}/../lib${LD_LIBRARY_PATH:+:${LD_LIBRARY_PATH}}" +exec "${BIN_DIR}/../libexec/proxysql-cli" "$@" +EOF +chmod 0755 "pkgroot/${DIR_NAME}/bin/proxysql-cli" + bundle_runtime_library() { local soname="$1" local resolved_path diff --git a/test/infra/control/test-verify-package-install.bash b/test/infra/control/test-verify-package-install.bash index a42304ad44..e9c7e74cfe 100755 --- a/test/infra/control/test-verify-package-install.bash +++ b/test/infra/control/test-verify-package-install.bash @@ -62,6 +62,14 @@ cat >"$FAKE_BIN/proxysql" <<'EOF' echo 'ProxySQL version 3.1.11' EOF +# proxysql-cli is the same binary under a second name; the verifier checks it +# is on PATH and dispatches to CLI mode. +cat >"$FAKE_BIN/proxysql-cli" <<'EOF' +#!/usr/bin/env bash +echo 'Usage: proxysql-cli [options]' >&2 +echo ' import-pgbouncer [--dry-run] [--ignore-warnings]' >&2 +EOF + cat >"$FAKE_BIN/od" <<'EOF' #!/usr/bin/env bash echo ' 7f 45 4c 46' diff --git a/test/infra/control/verify-package-install.bash b/test/infra/control/verify-package-install.bash index 413166465e..e135da584a 100755 --- a/test/infra/control/verify-package-install.bash +++ b/test/infra/control/verify-package-install.bash @@ -143,8 +143,28 @@ echo "==> Binary smoke test (proxysql --version)" proxysql --version 2>&1 | head -5 echo "" -echo "==> Plugin .so presence check" +echo "==> proxysql-cli check" ALL_OK=0 +# proxysql-cli is the same binary under a second name (main() dispatches on +# argv[0]). It is created by the rpm %install symlink and the deb postinst, so +# a packaging regression that drops it is invisible to the proxysql smoke test +# above -- check it explicitly. +if command -v proxysql-cli >/dev/null 2>&1; then + echo " OK proxysql-cli (on PATH at $(command -v proxysql-cli))" + if proxysql-cli help 2>&1 | grep -q "import-pgbouncer"; then + echo " OK proxysql-cli dispatches to CLI mode" + else + echo " FAIL proxysql-cli did not dispatch to CLI mode" >&2 + proxysql-cli help 2>&1 | head -5 >&2 + ALL_OK=1 + fi +else + echo " FAIL proxysql-cli not found on PATH" >&2 + ALL_OK=1 +fi +echo "" + +echo "==> Plugin .so presence check" if [[ "$HAS_PLUGINS" == "true" ]]; then for plugin in ProxySQL_MySQLX_Plugin.so ProxySQL_GenAI_Plugin.so; do path="/usr/lib/proxysql/${plugin}" diff --git a/test/tap/groups/groups.json b/test/tap/groups/groups.json index b5cea7b42d..0f3ec3f1f1 100644 --- a/test/tap/groups/groups.json +++ b/test/tap/groups/groups.json @@ -159,6 +159,9 @@ "ok_packet_mixed_queries-t" : [ "legacy-g10","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4","mysql84-g4","mysql90-g4","mysql95-g4" ], "parsersql_digest_test-t" : [ "unit-tests-g1" ], "parsersql_unit-t" : [ "unit-tests-g1" ], + "pgbouncer_config_parser_unit-t" : [ "unit-tests-g1" ], + "pgbouncer_converter_unit-t" : [ "unit-tests-g1" ], + "pgbouncer_show_commands_unit-t" : [ "unit-tests-g1" ], "pgsql-admin_metacmds-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-admin_metacmds_describe_all_tables-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-admin_metacmds_describe_queries-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], @@ -179,6 +182,7 @@ "pgsql-notice_test-t" : [ "legacy-g6","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ], "pgsql-options_startup_params-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-parameterized_kill_queries_test-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], + "pgsql-pgbouncer_compat-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-pool_churn-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-proxysql_cmd_test-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql-query_cache_soft_ttl_pct-t" : [ "legacy-g6","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ], diff --git a/test/tap/tests/pgsql-pgbouncer_compat-t.cpp b/test/tap/tests/pgsql-pgbouncer_compat-t.cpp new file mode 100644 index 0000000000..6ae3860dba --- /dev/null +++ b/test/tap/tests/pgsql-pgbouncer_compat-t.cpp @@ -0,0 +1,275 @@ +/** + * @file pgsql-pgbouncer_compat-t.cpp + * @brief Integration tests for the PgBouncer compatibility layer. + * + * The unit tests under test/tap/tests/unit/ compare generated strings. That is + * not enough: every SHOW translation and every converter INSERT is SQL that the + * admin interface has to execute, and a wrong column name is invisible to a + * string comparison. Three such defects shipped in the original branch + * (pgsql_query_rules.schemaname, stats_pgsql_processlist.db, and reading + * weight/max_connections from stats_pgsql_connection_pool). + * + * This test therefore executes, against a live PgSQL admin port: + * - every PgBouncer SHOW command, plain and EXTENDED + * - every SQL statement the converter emits for a representative config + * so that a schema drift or a typo fails here rather than in production. + */ + +#include +#include +#include +#include +#include + +#include "command_line.h" +#include "tap.h" +#include "utils.h" + +#include "PgBouncer_Config.h" +#include "PgBouncer_ConfigConverter.h" + +// Every command PgBouncer's SHOW interface exposes and we claim to support. +static const char* SUPPORTED_SHOW[] = { + "POOLS", "STATS", "SERVERS", "CLIENTS", "DATABASES", + "USERS", "CONFIG", "VERSION", "STATE", "LISTS" +}; +static const int NUM_SUPPORTED = sizeof(SUPPORTED_SHOW) / sizeof(SUPPORTED_SHOW[0]); + +// Commands we deliberately reject with an explanatory message. +static const char* UNSUPPORTED_SHOW[] = { + "DNS_HOSTS", "DNS_ZONES", "FDS", "PEERS", "PEER_POOLS", + "MEM", "ACTIVE_SOCKETS", "SOCKETS" +}; +static const int NUM_UNSUPPORTED = sizeof(UNSUPPORTED_SHOW) / sizeof(UNSUPPORTED_SHOW[0]); + +static PGconn* connect_admin(const CommandLine& cl) { + std::stringstream cs; + cs << "host=" << cl.pgsql_admin_host + << " port=" << cl.pgsql_admin_port + << " user=" << cl.admin_username + << " password=" << cl.admin_password + << " dbname=postgres"; + return PQconnectdb(cs.str().c_str()); +} + +// Run a statement, reporting the server's own error text on failure so a +// failing assertion names the offending column instead of just "failed". +static bool exec_ok(PGconn* c, const std::string& q, const std::string& what) { + PGresult* res = PQexec(c, q.c_str()); + ExecStatusType st = PQresultStatus(res); + bool passed = (st == PGRES_TUPLES_OK || st == PGRES_COMMAND_OK); + if (!passed) { + diag("%s failed: %s", what.c_str(), PQerrorMessage(c)); + diag(" statement: %s", q.c_str()); + } + PQclear(res); + return passed; +} + +// --------------------------------------------------------------------------- +// SHOW command execution +// --------------------------------------------------------------------------- +static void test_show_commands(PGconn* admin) { + for (int i = 0; i < NUM_SUPPORTED; i++) { + std::string q = std::string("SHOW ") + SUPPORTED_SHOW[i]; + ok(exec_ok(admin, q, q), "%s executes", q.c_str()); + } + for (int i = 0; i < NUM_SUPPORTED; i++) { + std::string q = std::string("SHOW EXTENDED ") + SUPPORTED_SHOW[i]; + ok(exec_ok(admin, q, q), "%s executes", q.c_str()); + } +} + +// SHOW EXTENDED must return at least as many columns as the plain form, and +// strictly more for the commands that have ProxySQL-specific data to add. +static void test_extended_adds_columns(PGconn* admin) { + for (int i = 0; i < NUM_SUPPORTED; i++) { + std::string plain = std::string("SHOW ") + SUPPORTED_SHOW[i]; + std::string ext = std::string("SHOW EXTENDED ") + SUPPORTED_SHOW[i]; + + PGresult* r1 = PQexec(admin, plain.c_str()); + PGresult* r2 = PQexec(admin, ext.c_str()); + int n1 = (PQresultStatus(r1) == PGRES_TUPLES_OK) ? PQnfields(r1) : -1; + int n2 = (PQresultStatus(r2) == PGRES_TUPLES_OK) ? PQnfields(r2) : -1; + PQclear(r1); + PQclear(r2); + + ok(n1 > 0 && n2 > n1, + "SHOW EXTENDED %s returns more columns than SHOW %s (%d > %d)", + SUPPORTED_SHOW[i], SUPPORTED_SHOW[i], n2, n1); + } +} + +static void test_unsupported_commands(PGconn* admin) { + for (int i = 0; i < NUM_UNSUPPORTED; i++) { + std::string q = std::string("SHOW ") + UNSUPPORTED_SHOW[i]; + PGresult* res = PQexec(admin, q.c_str()); + bool errored = (PQresultStatus(res) == PGRES_FATAL_ERROR); + std::string msg = PQerrorMessage(admin); + PQclear(res); + ok(errored && msg.find("not supported") != std::string::npos, + "%s is rejected with an explanatory message", q.c_str()); + } +} + +// A SHOW that is not part of the PgBouncer set must still reach ProxySQL's own +// handling rather than being swallowed by the translation layer. +static void test_native_show_still_works(PGconn* admin) { + ok(exec_ok(admin, "SHOW TABLES", "SHOW TABLES"), + "native SHOW TABLES is unaffected by the PgBouncer layer"); + ok(exec_ok(admin, "SHOW PGSQL VARIABLES", "SHOW PGSQL VARIABLES"), + "native SHOW PGSQL VARIABLES is unaffected by the PgBouncer layer"); + + // Trailing tokens are not a PgBouncer command; this must not translate. + PGresult* res = PQexec(admin, "SHOW POOLS bogus_trailing_token"); + bool rejected = (PQresultStatus(res) == PGRES_FATAL_ERROR); + PQclear(res); + ok(rejected, "SHOW POOLS is not treated as SHOW POOLS"); +} + +// --------------------------------------------------------------------------- +// Converter output must be executable SQL +// --------------------------------------------------------------------------- +static PgBouncer::Config build_representative_config() { + PgBouncer::Config config; + config.global.listen_port = 6432; + config.global.auth_type = "md5"; + config.global.pool_mode = "transaction"; + config.global.max_client_conn = 500; + config.global.default_pool_size = 25; + + PgBouncer::Database db; + db.name = "appdb"; + db.host = "127.0.0.1"; + db.port = 5432; + db.pool_size = 30; + config.databases.push_back(db); + + PgBouncer::Database wild; + wild.name = "*"; + wild.host = "127.0.0.1"; + wild.port = 5432; + config.databases.push_back(wild); + + PgBouncer::User user; + user.name = "appuser"; + user.pool_mode = "transaction"; + config.users.push_back(user); + + PgBouncer::AuthFileEntry auth; + auth.username = "appuser"; + auth.password = "appsecret"; + auth.type = PgBouncer::AuthType::PLAIN; + config.auth_entries.push_back(auth); + + PgBouncer::HBARule rule; + rule.conn_type = "host"; + rule.database = "all"; + rule.user = "all"; + rule.address = "10.0.0.0/8"; + rule.method = "md5"; + config.hba_rules.push_back(rule); + + return config; +} + +static void test_converter_sql_executes(PGconn* admin) { + PgBouncer::Config config = build_representative_config(); + PgBouncer::ConfigConverter converter; + PgBouncer::ConversionResult result = converter.convert(config, false); + + ok(!result.entries.empty(), "converter produced SQL for the sample config"); + + int executed = 0; + int failed = 0; + for (const auto& e : result.entries) { + // Comment-only entries carry no statement to run. + if (e.sql.empty() || e.sql.compare(0, 2, "--") == 0) continue; + executed++; + PGresult* res = PQexec(admin, e.sql.c_str()); + ExecStatusType st = PQresultStatus(res); + if (st != PGRES_COMMAND_OK && st != PGRES_TUPLES_OK) { + failed++; + diag("converter SQL failed: %s", e.sql.c_str()); + diag(" server said: %s", PQerrorMessage(admin)); + } + PQclear(res); + } + + ok(executed > 0, "converter SQL contained executable statements (%d)", executed); + ok(failed == 0, "every converter statement executed cleanly (%d failed of %d)", + failed, executed); +} + +// After the import the tables it targets must actually hold the imported rows. +static void test_converter_populated_tables(PGconn* admin) { + struct { const char* table; const char* what; } checks[] = { + { "pgsql_servers", "servers" }, + { "pgsql_users", "users" }, + { "pgsql_query_rules", "query rules" }, + }; + for (const auto& c : checks) { + std::string q = std::string("SELECT COUNT(*) FROM ") + c.table; + PGresult* res = PQexec(admin, q.c_str()); + bool has_rows = false; + if (PQresultStatus(res) == PGRES_TUPLES_OK && PQntuples(res) == 1) { + has_rows = atoi(PQgetvalue(res, 0, 0)) > 0; + } + PQclear(res); + ok(has_rows, "%s were imported into %s", c.what, c.table); + } +} + +// IMPORT with a path that does not exist must fail cleanly, not crash or hang. +static void test_import_missing_file(PGconn* admin) { + PGresult* res = PQexec(admin, + "IMPORT PGBOUNCER CONFIG FROM '/nonexistent/pgbouncer.ini' DRY RUN"); + bool errored = (PQresultStatus(res) == PGRES_FATAL_ERROR); + PQclear(res); + ok(errored, "IMPORT PGBOUNCER CONFIG from a missing file reports an error"); + + // The connection must still be usable afterwards. + ok(exec_ok(admin, "SELECT 1", "SELECT 1"), + "admin connection survives a failed IMPORT"); +} + +int main(int argc, char** argv) { + CommandLine cl; + + if (cl.getEnv()) { + diag("Failed to get the required environmental variables."); + return -1; + } + + // 10 plain SHOW + 10 EXTENDED SHOW + 10 column-count + 8 unsupported + // + 3 native SHOW + 3 converter SQL + 3 populated tables + 2 import + plan(NUM_SUPPORTED * 3 + NUM_UNSUPPORTED + 3 + 3 + 3 + 2); + + PGconn* admin = connect_admin(cl); + if (PQstatus(admin) != CONNECTION_OK) { + diag("Failed to connect to the PgSQL admin interface: %s", PQerrorMessage(admin)); + PQfinish(admin); + return exit_status(); + } + + test_show_commands(admin); + test_extended_adds_columns(admin); + test_unsupported_commands(admin); + test_native_show_still_works(admin); + + test_converter_sql_executes(admin); + test_converter_populated_tables(admin); + test_import_missing_file(admin); + + // Put the runtime configuration back the way we found it: the converter + // statements above rewrote pgsql_servers/users/query_rules in memory. + PQclear(PQexec(admin, "LOAD PGSQL SERVERS FROM DISK")); + PQclear(PQexec(admin, "LOAD PGSQL USERS FROM DISK")); + PQclear(PQexec(admin, "LOAD PGSQL QUERY RULES FROM DISK")); + PQclear(PQexec(admin, "LOAD PGSQL SERVERS TO RUNTIME")); + PQclear(PQexec(admin, "LOAD PGSQL USERS TO RUNTIME")); + PQclear(PQexec(admin, "LOAD PGSQL QUERY RULES TO RUNTIME")); + + PQfinish(admin); + return exit_status(); +} From 57da099469ed532d9b9179a20fd3c37232222139 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Thu, 27 Aug 2026 07:01:38 +0000 Subject: [PATCH 09/12] fix(pgbouncer): keep the integration test from clobbering the shared instance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running pgsql-pgbouncer_compat-t destroyed the test instance's PostgreSQL configuration, and every test that ran afterwards failed with PgSQL_Session.cpp:4184 [ERROR] ProxySQL Error: Access denied for user 'postgres'@'...' (using password: NO) because pgsql_users and pgsql_servers had been emptied. Cause: the converter's output ends with SAVE PGSQL {SERVERS,USERS,QUERY RULES, VARIABLES} TO DISK, and the test executed every statement it produced. Those SAVEs overwrote the instance's on-disk configuration, so the test's own "LOAD ... FROM DISK" restore then faithfully restored the clobbered copy. The data directory is a host bind-mount, so recreating the ProxySQL container did not recover it either — the damage outlived the container. The test now skips the SAVE statements (they are fixed SQL with no generated identifiers, so no mapping coverage is lost) and asserts they were emitted. Everything else it runs touches memory and runtime only, which leaves the on-disk copy a valid source to restore from. The restore is widened to cover variables and the firewall whitelist, and reports a failed restore via diag() so a broken teardown is visible rather than silent. Also adds the cheap "SHOW " prefix guard around the PgBouncer block in Admin_Handler. Now that the block sits ahead of the generic SHOW dispatch, it would otherwise normalize and tokenize every admin query, not just SHOW ones. Verified against a live legacy-g4 instance: - pgsql-proxysql_cmd_test-t passes before and after pgsql-pgbouncer_compat-t in the same session (it failed on the second run before this fix) - pgsql_users/pgsql_servers still hold the infra's own rows afterwards - 6/6 of the admin + pgsql regression set pass - pgsql-pgbouncer_compat-t: 49/49 --- lib/Admin_Handler.cpp | 4 ++ test/tap/tests/pgsql-pgbouncer_compat-t.cpp | 58 +++++++++++++++++---- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/lib/Admin_Handler.cpp b/lib/Admin_Handler.cpp index 230149ed84..4b7076beda 100644 --- a/lib/Admin_Handler.cpp +++ b/lib/Admin_Handler.cpp @@ -5139,6 +5139,9 @@ void admin_session_handler(S* sess, void *_pa, PtrSize_t *pkt) { // through to the normal handling. // ========================================================================= if constexpr (std::is_same_v) { + // Cheap gate: this block now sits ahead of the generic SHOW dispatch, so + // without it every admin query would be normalized and tokenized twice. + if (query_no_space_length > 5 && !strncasecmp("SHOW ", query_no_space, 5)) { // Check for unsupported PgBouncer SHOW commands first std::string unsupported_msg = PgBouncer::get_unsupported_show_message(query_no_space, query_no_space_length); if (!unsupported_msg.empty()) { @@ -5157,6 +5160,7 @@ void admin_session_handler(S* sess, void *_pa, PtrSize_t *pkt) { query_length = strlen(query) + 1; goto __run_query; } + } } if (strncasecmp("SHOW ", query_no_space, 5)) { diff --git a/test/tap/tests/pgsql-pgbouncer_compat-t.cpp b/test/tap/tests/pgsql-pgbouncer_compat-t.cpp index 6ae3860dba..af9cc63106 100644 --- a/test/tap/tests/pgsql-pgbouncer_compat-t.cpp +++ b/test/tap/tests/pgsql-pgbouncer_compat-t.cpp @@ -182,9 +182,23 @@ static void test_converter_sql_executes(PGconn* admin) { int executed = 0; int failed = 0; + int skipped_persist = 0; for (const auto& e : result.entries) { // Comment-only entries carry no statement to run. if (e.sql.empty() || e.sql.compare(0, 2, "--") == 0) continue; + + // Never execute the converter's "SAVE ... TO DISK" statements. They + // would overwrite this instance's on-disk configuration permanently, + // which no LOAD ... FROM DISK could then undo -- it would restore the + // clobbered copy. Every other statement touches memory/runtime only, + // so the restore at the end of main() puts things back. The SAVE + // statements are plain, fixed SQL with no generated identifiers, so + // skipping them costs no coverage of the mapping logic. + if (strncasecmp(e.sql.c_str(), "SAVE ", 5) == 0) { + skipped_persist++; + continue; + } + executed++; PGresult* res = PQexec(admin, e.sql.c_str()); ExecStatusType st = PQresultStatus(res); @@ -199,6 +213,9 @@ static void test_converter_sql_executes(PGconn* admin) { ok(executed > 0, "converter SQL contained executable statements (%d)", executed); ok(failed == 0, "every converter statement executed cleanly (%d failed of %d)", failed, executed); + ok(skipped_persist > 0, + "converter emits SAVE ... TO DISK statements (%d, deliberately not executed here)", + skipped_persist); } // After the import the tables it targets must actually hold the imported rows. @@ -242,8 +259,8 @@ int main(int argc, char** argv) { } // 10 plain SHOW + 10 EXTENDED SHOW + 10 column-count + 8 unsupported - // + 3 native SHOW + 3 converter SQL + 3 populated tables + 2 import - plan(NUM_SUPPORTED * 3 + NUM_UNSUPPORTED + 3 + 3 + 3 + 2); + // + 3 native SHOW + 4 converter SQL + 3 populated tables + 2 import + plan(NUM_SUPPORTED * 3 + NUM_UNSUPPORTED + 3 + 4 + 3 + 2); PGconn* admin = connect_admin(cl); if (PQstatus(admin) != CONNECTION_OK) { @@ -261,14 +278,35 @@ int main(int argc, char** argv) { test_converter_populated_tables(admin); test_import_missing_file(admin); - // Put the runtime configuration back the way we found it: the converter - // statements above rewrote pgsql_servers/users/query_rules in memory. - PQclear(PQexec(admin, "LOAD PGSQL SERVERS FROM DISK")); - PQclear(PQexec(admin, "LOAD PGSQL USERS FROM DISK")); - PQclear(PQexec(admin, "LOAD PGSQL QUERY RULES FROM DISK")); - PQclear(PQexec(admin, "LOAD PGSQL SERVERS TO RUNTIME")); - PQclear(PQexec(admin, "LOAD PGSQL USERS TO RUNTIME")); - PQclear(PQexec(admin, "LOAD PGSQL QUERY RULES TO RUNTIME")); + // Put the configuration back the way we found it. The converter statements + // above rewrote pgsql_servers / pgsql_users / pgsql_query_rules / the + // firewall whitelist and set pgsql-* variables, all in memory only -- the + // SAVE ... TO DISK statements were deliberately skipped, so the on-disk + // copy is still the one this instance was configured with and is a valid + // source to restore from. Other tests share this instance; leaving it + // holding a PgBouncer import would break them. + static const char* RESTORE[] = { + "LOAD PGSQL SERVERS FROM DISK", + "LOAD PGSQL USERS FROM DISK", + "LOAD PGSQL QUERY RULES FROM DISK", + "LOAD PGSQL VARIABLES FROM DISK", + "LOAD PGSQL FIREWALL FROM DISK", + "LOAD PGSQL SERVERS TO RUNTIME", + "LOAD PGSQL USERS TO RUNTIME", + "LOAD PGSQL QUERY RULES TO RUNTIME", + "LOAD PGSQL VARIABLES TO RUNTIME", + "LOAD PGSQL FIREWALL TO RUNTIME", + }; + for (const char* stmt : RESTORE) { + PGresult* res = PQexec(admin, stmt); + ExecStatusType st = PQresultStatus(res); + if (st != PGRES_COMMAND_OK && st != PGRES_TUPLES_OK) { + // Not an assertion (the plan is fixed), but it must be visible: + // a failed restore leaves the shared instance misconfigured. + diag("RESTORE FAILED: %s -> %s", stmt, PQerrorMessage(admin)); + } + PQclear(res); + } PQfinish(admin); return exit_status(); From 5f61301cdf81da13bb8ce38b4ccfcdf5d842d005 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Thu, 27 Aug 2026 08:48:51 +0000 Subject: [PATCH 10/12] fix(pgbouncer): map auth_type instead of silently dropping it auth_type was parsed into the Config and then ignored: the converter never read it and check_unmappable() never mentioned it. The frontend authentication method of the pooler being replaced simply vanished from the import, with no error even in strict mode -- which contradicts the converter's own strict-by-default contract, and does so for the most security-relevant setting in pgbouncer.ini. It maps cleanly onto the existing pgsql-authentication_method variable (PgSQL_Thread.cpp, range 1-3): plain / password -> 1 (cleartext) md5 -> 2 scram-sha-256 -> 3 The values ProxySQL cannot express are now reported (fatal in strict mode): trust and any (ProxySQL always verifies the user against pgsql_users), hba (pgsql-authentication_method is global, so per-rule pg_hba.conf methods cannot select the frontend method), and cert / pam. Also records in doc/PGBOUNCER_COMPAT.md that the pre-hashed-password limitation is expected to lift with #5865 / #5863, which teaches pgsql_users.password to hold a SCRAM verifier or md5 hash directly -- the same formats userlist.txt already stores -- together with the constraints that come with it (md5 secret needs an md5 backend, verifier needs a scram-sha-256 backend and must be byte-identical to the backend's rolpassword). Converter unit tests: 52 -> 68. --- doc/PGBOUNCER_COMPAT.md | 15 ++++ .../PgBouncer_ConfigConverter.cpp | 40 +++++++++++ .../tests/unit/pgbouncer_converter_unit-t.cpp | 68 ++++++++++++++++++- 3 files changed, 122 insertions(+), 1 deletion(-) diff --git a/doc/PGBOUNCER_COMPAT.md b/doc/PGBOUNCER_COMPAT.md index 7bd863550a..a46f317377 100644 --- a/doc/PGBOUNCER_COMPAT.md +++ b/doc/PGBOUNCER_COMPAT.md @@ -87,6 +87,7 @@ rather than a silently truncated field. | PgBouncer | ProxySQL | Note | |---|---|---| | `listen_addr` + `listen_port` | `pgsql-interfaces` | | +| `auth_type` | `pgsql-authentication_method` | `plain`/`password`→1, `md5`→2, `scram-sha-256`→3 | | `max_client_conn` | `pgsql-max_connections` | | | `server_connect_timeout` | `pgsql-connect_timeout_server` | | | `server_lifetime` | `pgsql-connection_max_age_ms` | s → ms | @@ -138,12 +139,26 @@ These are reported per occurrence — fatal in strict mode, warnings otherwise. **Authentication.** `auth_query`, `auth_user`, `auth_dbname`. ProxySQL authenticates from `pgsql_users` (or LDAP), not by querying the backend. +**`auth_type` values with no equivalent.** `trust` and `any` (ProxySQL always +verifies the user against `pgsql_users` and cannot accept an unauthenticated +connection), `hba` (`pgsql-authentication_method` is global, so per-rule methods +from `pg_hba.conf` cannot select the frontend method), and `cert`/`pam`. + **Pre-hashed passwords.** A `userlist.txt` entry holding an MD5 or SCRAM verifier is imported verbatim but **will not authenticate**. ProxySQL derives both the MD5 challenge response and the SCRAM verifier from the *cleartext* password stored in `pgsql_users.password`, so a pre-hashed value cannot be used. Replace those entries with the cleartext password after importing. +> This limitation is expected to lift with PR #5865 / issue #5863, which teaches +> `pgsql_users.password` to hold a SCRAM verifier or an `md5…` hash directly — +> exactly the formats `userlist.txt` already stores. When that lands, this +> section and the strict-mode error in `convert_users()` should be revisited so a +> `userlist.txt` imports as-is. Note the constraints that come with it: an +> md5-stored user needs an md5 backend, a verifier-stored user needs a +> `scram-sha-256` backend, and the verifier must be byte-identical to the +> backend's `rolpassword` (same salt and iterations). + **`dbname=` aliases.** PgBouncer's `dbname=` connects to a backend database under a different name than the client asked for. ProxySQL routes to a hostgroup but does not rewrite the database in the startup packet, so the alias cannot be diff --git a/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp b/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp index 43e8401ec6..e7d9cd9064 100644 --- a/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp +++ b/lib/pgbouncer_compat/PgBouncer_ConfigConverter.cpp @@ -1,6 +1,7 @@ #include "PgBouncer_ConfigConverter.h" #include #include +#include #include namespace PgBouncer { @@ -293,6 +294,45 @@ void ConfigConverter::convert_globals(const Config& config, "PgBouncer listen_addr:listen_port -> ProxySQL pgsql-interfaces"); } + // -- auth_type -> pgsql-authentication_method + // + // pgsql-authentication_method selects the method ProxySQL challenges + // clients with: 1 = cleartext, 2 = md5, 3 = scram-sha-256. + { + std::string at = g.auth_type; + std::transform(at.begin(), at.end(), at.begin(), + [](unsigned char c) { return std::tolower(c); }); + + if (at == "plain" || at == "password") { + emit_set_int("pgsql-authentication_method", 1, + "PgBouncer auth_type=" + g.auth_type + + " -> ProxySQL cleartext authentication"); + } else if (at == "md5") { + emit_set_int("pgsql-authentication_method", 2, + "PgBouncer auth_type=md5 -> ProxySQL md5 authentication"); + } else if (at == "scram-sha-256") { + emit_set_int("pgsql-authentication_method", 3, + "PgBouncer auth_type=scram-sha-256 -> ProxySQL SCRAM authentication"); + } else if (at == "trust") { + add_issue(result, strict, + "auth_type=trust has no ProxySQL equivalent; ProxySQL always " + "authenticates clients against pgsql_users and cannot accept " + "unauthenticated connections"); + } else if (at == "hba") { + add_issue(result, strict, + "auth_type=hba has no ProxySQL equivalent; the per-rule methods " + "in pg_hba.conf cannot select the frontend authentication method, " + "which is global (pgsql-authentication_method)"); + } else if (at == "any") { + add_issue(result, strict, + "auth_type=any has no ProxySQL equivalent; ProxySQL always " + "verifies the username against pgsql_users"); + } else if (!at.empty()) { + add_issue(result, strict, + "auth_type=" + g.auth_type + " has no ProxySQL equivalent"); + } + } + // -- max_client_conn -> pgsql-max_connections (ProxySQL default: 2048) if (g.max_client_conn != 100) { // PgBouncer default is 100; only emit if changed diff --git a/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp b/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp index 3ea7ff7f83..3a2391b3e5 100644 --- a/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp +++ b/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp @@ -441,8 +441,73 @@ void test_dbname_alias_is_reported() { CHECK(!strict.success, "dbname alias fails the import in strict mode"); } + +// ============================================================ +// Test: auth_type is mapped, not silently dropped +// +// Regression: auth_type was parsed into the Config and then ignored by the +// converter entirely -- neither mapped nor reported. The frontend +// authentication method of the pooler being replaced simply vanished, which +// under the converter's strict-by-default contract should never happen. +// ============================================================ +static PgBouncer::Config config_with_auth_type(const char* auth_type) { + PgBouncer::Config config; + PgBouncer::Database db; + db.name = "mydb"; + db.host = "10.0.0.1"; + config.databases.push_back(db); + config.global.auth_type = auth_type; + return config; +} + +void test_auth_type_mapping() { + // Match the whole emitted statement, so a wrong level fails the assertion + // rather than passing on an incidental substring. + struct { const char* auth_type; const char* expect_sql; } mapped[] = { + { "plain", "SET pgsql-authentication_method=1;" }, + { "password", "SET pgsql-authentication_method=1;" }, + { "md5", "SET pgsql-authentication_method=2;" }, + { "scram-sha-256", "SET pgsql-authentication_method=3;" }, + }; + for (const auto& m : mapped) { + PgBouncer::Config config = config_with_auth_type(m.auth_type); + PgBouncer::ConfigConverter conv; + PgBouncer::ConversionResult r = conv.convert(config, false); + std::string msg = std::string("auth_type=") + m.auth_type + " emits " + m.expect_sql; + CHECK(has_sql_containing(r, m.expect_sql), msg.c_str()); + } + + // A mapped auth_type must not also be reported as a problem. + { + PgBouncer::ConfigConverter conv; + PgBouncer::ConversionResult r = conv.convert(config_with_auth_type("md5"), true); + CHECK(r.success, "a mappable auth_type does not fail strict mode"); + } + + // Case-insensitive, matching the rest of the INI handling. + { + PgBouncer::ConfigConverter conv; + PgBouncer::ConversionResult r = conv.convert(config_with_auth_type("MD5"), false); + CHECK(has_sql_containing(r, "pgsql-authentication_method"), + "auth_type matching is case-insensitive"); + } + + // The methods ProxySQL cannot express are reported, not dropped. + for (const char* unmappable : { "trust", "hba", "any", "cert", "pam" }) { + PgBouncer::ConfigConverter conv; + PgBouncer::ConversionResult r = conv.convert(config_with_auth_type(unmappable), false); + std::string msg = std::string("auth_type=") + unmappable + " is reported as unmappable"; + CHECK(has_issue_containing(r, unmappable), msg.c_str()); + + PgBouncer::ConfigConverter sconv; + PgBouncer::ConversionResult sr = sconv.convert(config_with_auth_type(unmappable), true); + std::string smsg = std::string("auth_type=") + unmappable + " fails the import in strict mode"; + CHECK(!sr.success, smsg.c_str()); + } +} + int main() { - plan(52); + plan(68); test_minimal_conversion(); // 6 test_multi_host_conversion(); // 5 @@ -459,6 +524,7 @@ int main() { test_hba_reject_is_reported(); test_hashed_password_is_flagged(); test_dbname_alias_is_reported(); + test_auth_type_mapping(); return exit_status(); } From 4bf6105fab7a3d240b0996253ae8d90f855aa2d4 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Thu, 27 Aug 2026 08:52:08 +0000 Subject: [PATCH 11/12] docs(pgbouncer): link the pre-hashed-password limitation to its tracking issue The note about hashed userlist.txt credentials now points at #6134, which records what to change in convert_users() and in the tests once #5865 lands. --- doc/PGBOUNCER_COMPAT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/PGBOUNCER_COMPAT.md b/doc/PGBOUNCER_COMPAT.md index a46f317377..e8a9cc04d5 100644 --- a/doc/PGBOUNCER_COMPAT.md +++ b/doc/PGBOUNCER_COMPAT.md @@ -150,7 +150,7 @@ challenge response and the SCRAM verifier from the *cleartext* password stored i `pgsql_users.password`, so a pre-hashed value cannot be used. Replace those entries with the cleartext password after importing. -> This limitation is expected to lift with PR #5865 / issue #5863, which teaches +> Tracked in #6134. This limitation is expected to lift with PR #5865 / issue #5863, which teaches > `pgsql_users.password` to hold a SCRAM verifier or an `md5…` hash directly — > exactly the formats `userlist.txt` already stores. When that lands, this > section and the strict-mode error in `convert_users()` should be revisited so a From 10c489321bfa5677ca07f7f5be8cd08beee95a4f Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Thu, 27 Aug 2026 09:06:25 +0000 Subject: [PATCH 12/12] test(pgbouncer): annotate synthetic test credentials for SonarCloud SonarCloud's quality gate failed on the PR with an E security rating from seven cpp:S2068 "hard-coded password" findings, all of them in the new test files. Each is a synthetic fixture value assigned to a PgBouncer::AuthFileEntry built in memory -- the md5 one is the hash of the empty string, and the SCRAM one is a structurally-valid but meaningless verifier whose salt and keys are base64 "salt"/"str". None is a real credential and none reaches a credential store; two of them exist precisely to assert that a hashed credential is reported as unusable. Annotated with // NOSONAR(cpp:S2068) plus a per-line reason, following the convention already used for the same rule in test/tap/tests/reg_test_5988-caching_sha2_rsa-t.cpp:168. Tests rebuilt and re-run: pgbouncer_converter_unit-t 68/68, pgsql-pgbouncer_compat-t links clean. --- test/tap/tests/pgsql-pgbouncer_compat-t.cpp | 2 +- test/tap/tests/unit/pgbouncer_converter_unit-t.cpp | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/tap/tests/pgsql-pgbouncer_compat-t.cpp b/test/tap/tests/pgsql-pgbouncer_compat-t.cpp index af9cc63106..513cf08883 100644 --- a/test/tap/tests/pgsql-pgbouncer_compat-t.cpp +++ b/test/tap/tests/pgsql-pgbouncer_compat-t.cpp @@ -158,7 +158,7 @@ static PgBouncer::Config build_representative_config() { PgBouncer::AuthFileEntry auth; auth.username = "appuser"; - auth.password = "appsecret"; + auth.password = "appsecret"; // NOSONAR(cpp:S2068): synthetic fixture value for an in-memory PgBouncer::Config; never reaches a real credential store. auth.type = PgBouncer::AuthType::PLAIN; config.auth_entries.push_back(auth); diff --git a/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp b/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp index 3a2391b3e5..8160964abe 100644 --- a/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp +++ b/test/tap/tests/unit/pgbouncer_converter_unit-t.cpp @@ -88,7 +88,7 @@ void test_wildcard_database() { PgBouncer::AuthFileEntry auth; auth.username = "testuser"; - auth.password = "secret"; + auth.password = "secret"; // NOSONAR(cpp:S2068): synthetic fixture value; the converter is exercised entirely in memory. auth.type = PgBouncer::AuthType::PLAIN; config.auth_entries.push_back(auth); @@ -110,13 +110,13 @@ void test_user_pool_mode_mapping() { PgBouncer::AuthFileEntry auth1; auth1.username = "session_user"; - auth1.password = "pass1"; + auth1.password = "pass1"; // NOSONAR(cpp:S2068): synthetic fixture value. auth1.type = PgBouncer::AuthType::PLAIN; config.auth_entries.push_back(auth1); PgBouncer::AuthFileEntry auth2; auth2.username = "txn_user"; - auth2.password = "pass2"; + auth2.password = "pass2"; // NOSONAR(cpp:S2068): synthetic fixture value. auth2.type = PgBouncer::AuthType::PLAIN; config.auth_entries.push_back(auth2); @@ -387,19 +387,19 @@ void test_hashed_password_is_flagged() { PgBouncer::AuthFileEntry md5e; md5e.username = "alice"; - md5e.password = "md5d41d8cd98f00b204e9800998ecf8427"; + md5e.password = "md5d41d8cd98f00b204e9800998ecf8427"; // NOSONAR(cpp:S2068): md5 of the empty string, used to assert the MD5 verifier is reported as unusable. md5e.type = PgBouncer::AuthType::MD5; config.auth_entries.push_back(md5e); PgBouncer::AuthFileEntry scram; scram.username = "bob"; - scram.password = "SCRAM-SHA-256$4096:c2FsdA==$c3Ry:c3Ry"; + scram.password = "SCRAM-SHA-256$4096:c2FsdA==$c3Ry:c3Ry"; // NOSONAR(cpp:S2068): structurally-valid but meaningless verifier ("salt"/"str" base64), used to assert SCRAM is reported as unusable. scram.type = PgBouncer::AuthType::SCRAM; config.auth_entries.push_back(scram); PgBouncer::AuthFileEntry plain; plain.username = "carol"; - plain.password = "secret"; + plain.password = "secret"; // NOSONAR(cpp:S2068): synthetic fixture value; the control case that must NOT be reported. plain.type = PgBouncer::AuthType::PLAIN; config.auth_entries.push_back(plain);