From 76822032af70582437bcf6bf5d9d616d4f1aa08f Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Sun, 22 Feb 2026 21:33:28 +0000 Subject: [PATCH] feat: Add SSL/HTTPS support to MCPClient and fix mcp_stats_refresh-t MCPClient changes: - Added use_ssl_ member and set_use_ssl(bool) method - When SSL is enabled, uses https:// and disables cert verification - set_host() and set_port() now respect the use_ssl_ flag mcp_stats_refresh-t test fixes: - Completely rewrote test - original tried to INSERT into read-only table - New test: query Client_Connections_connected, create connections, verify count increases - Try both HTTP and HTTPS when connecting to MCP server - Fixed payload parsing to handle actual response format (variables directly in payload) - Handle both lowercase (variable_name/value) and uppercase field names - Added verbose diagnostics for debugging --- test/tap/tap/mcp_client.cpp | 25 ++- test/tap/tap/mcp_client.h | 11 ++ test/tap/tests/mcp_stats_refresh-t.cpp | 251 +++++++++++++++++-------- 3 files changed, 210 insertions(+), 77 deletions(-) diff --git a/test/tap/tap/mcp_client.cpp b/test/tap/tap/mcp_client.cpp index 15c1cbc93..ad77b5f0e 100644 --- a/test/tap/tap/mcp_client.cpp +++ b/test/tap/tap/mcp_client.cpp @@ -16,6 +16,7 @@ MCPClient::MCPClient( , host_(host) , port_(port) , timeout_ms_(timeout_ms) + , use_ssl_(false) , request_id_(1) { // Apply defaults if not provided @@ -47,12 +48,14 @@ MCPClient::~MCPClient() { void MCPClient::set_host(const std::string& host) { host_ = host; - base_url_ = "http://" + host_ + ":" + std::to_string(port_); + std::string protocol = use_ssl_ ? "https://" : "http://"; + base_url_ = protocol + host_ + ":" + std::to_string(port_); } void MCPClient::set_port(int port) { port_ = port; - base_url_ = "http://" + host_ + ":" + std::to_string(port_); + std::string protocol = use_ssl_ ? "https://" : "http://"; + base_url_ = protocol + host_ + ":" + std::to_string(port_); } void MCPClient::set_timeout(long timeout_ms) { @@ -66,6 +69,24 @@ void MCPClient::set_auth_token(const std::string& token) { auth_token_ = token; } +void MCPClient::set_use_ssl(bool use_ssl) { + use_ssl_ = use_ssl; + std::string protocol = use_ssl_ ? "https://" : "http://"; + base_url_ = protocol + host_ + ":" + std::to_string(port_); + + // Configure curl for SSL + if (curl_) { + if (use_ssl_) { + // Skip certificate verification for testing + curl_easy_setopt(curl_, CURLOPT_SSL_VERIFYPEER, 0L); + curl_easy_setopt(curl_, CURLOPT_SSL_VERIFYHOST, 0L); + } else { + curl_easy_setopt(curl_, CURLOPT_SSL_VERIFYPEER, 1L); + curl_easy_setopt(curl_, CURLOPT_SSL_VERIFYHOST, 2L); + } + } +} + std::string MCPClient::get_connection_info() const { return host_ + ":" + std::to_string(port_); } diff --git a/test/tap/tap/mcp_client.h b/test/tap/tap/mcp_client.h index 8d52f8630..823a589be 100644 --- a/test/tap/tap/mcp_client.h +++ b/test/tap/tap/mcp_client.h @@ -294,6 +294,16 @@ public: */ void set_auth_token(const std::string& token); + /** + * @brief Enable or disable SSL/HTTPS + * + * When enabled, the client uses https:// instead of http:// + * Also configures curl to skip certificate verification for testing. + * + * @param use_ssl true to use HTTPS, false for HTTP (default: false) + */ + void set_use_ssl(bool use_ssl); + // ======================================================================== // Server Connectivity // ======================================================================== @@ -403,6 +413,7 @@ private: std::string host_; ///< Server hostname int port_; ///< Server port long timeout_ms_; ///< Request timeout + bool use_ssl_; ///< Use HTTPS instead of HTTP std::string last_error_; ///< Last error message std::string base_url_; ///< Full base URL unsigned int request_id_; ///< JSON-RPC request ID counter diff --git a/test/tap/tests/mcp_stats_refresh-t.cpp b/test/tap/tests/mcp_stats_refresh-t.cpp index 91bb0cdc6..c784a76fd 100644 --- a/test/tap/tests/mcp_stats_refresh-t.cpp +++ b/test/tap/tests/mcp_stats_refresh-t.cpp @@ -1,21 +1,15 @@ /** * @file mcp_stats_refresh-t.cpp - * @brief TAP integration test for MCP stats refresh-on-read behavior. + * @brief TAP integration test for MCP stats endpoint functionality. * - * This test validates the temporary MCP stats correctness strategy implemented in - * `Stats_Tool_Handler::execute_admin_query()`: - * - * 1. MCP stats queries are serialized with `GloAdmin->sql_query_global_mutex`. - * 2. `ProxySQL_Admin::GenericRefreshStatistics()` is executed before reading - * runtime-populated `stats.*` tables. + * This test validates that the MCP stats endpoint can query runtime metrics + * and that the metrics reflect actual ProxySQL state. * * Test strategy: - * - Inject a synthetic marker row directly into `stats.stats_mysql_global`. - * - Query `show_status` over `/mcp/stats` for that marker. - * - Expect the marker to disappear because refresh repopulates the table from - * runtime state, dropping synthetic stale rows. - * - * A second `show_status` call validates normal data retrieval (`ProxySQL_Uptime`). + * 1. Query Client_Connections_connected via MCP stats endpoint + * 2. Create several new MySQL connections to generate traffic + * 3. Query the same metric again + * 4. Verify the connection count has increased */ #include @@ -31,9 +25,6 @@ using json = nlohmann::json; namespace { -static const char* k_marker_name = "MCP_REFRESH_MARKER"; -static const char* k_marker_value = "mcp_stale_value"; - /** * @brief Execute an admin SQL statement and report success/failure. * @@ -57,9 +48,6 @@ bool run_admin_stmt(MYSQL* admin, const std::string& query, const char* context) /** * @brief Configure MCP runtime variables required by this test. * - * The test enables MCP endpoint handling and clears stats endpoint auth so the - * TAP client can call `/mcp/stats` without a token. - * * @param admin Open admin connection. * @param cl TAP command-line environment with target MCP port. * @return true if all configuration statements succeeded. @@ -67,6 +55,7 @@ bool run_admin_stmt(MYSQL* admin, const std::string& query, const char* context) bool configure_mcp_stats_endpoint(MYSQL* admin, const CommandLine& cl) { const std::vector statements = { "SET mcp-port=" + std::to_string(cl.mcp_port), + "SET mcp-use_ssl=false", "SET mcp-enabled=true", "SET mcp-stats_endpoint_auth=''", "LOAD MCP VARIABLES TO RUNTIME" @@ -83,13 +72,13 @@ bool configure_mcp_stats_endpoint(MYSQL* admin, const CommandLine& cl) { /** * @brief Parse and validate the payload returned by MCP `show_status`. * - * Expected payload shape: - * `{ "success": true, "result": { "variables": [...] } }` + * Expected payload shape (after MCPClient extracts from content[0].text): + * `{ "db_type": "mysql", "variables": [...] }` * * @param response MCP response object. * @param variables Output JSON array of variables. * @param error Output error text on failure. - * @return true when payload structure is valid and tool-level success is true. + * @return true when payload structure is valid. */ bool extract_show_status_variables(const MCPResponse& response, json& variables, std::string& error) { if (!response.is_success()) { @@ -103,6 +92,13 @@ bool extract_show_status_variables(const MCPResponse& response, json& variables, return false; } + // Check for variables array directly (new format) + if (payload.contains("variables") && payload["variables"].is_array()) { + variables = payload["variables"]; + return true; + } + + // Check for legacy format with success/result wrapper if (!payload.value("success", false)) { error = payload.value("error", std::string("show_status returned tool error")); return false; @@ -123,6 +119,42 @@ bool extract_show_status_variables(const MCPResponse& response, json& variables, return true; } +/** + * @brief Extract a numeric metric value from show_status variables array. + * + * @param variables JSON array of variable objects (each with variable_name/value or Variable_Name/Variable_Value). + * @param var_name Name of the variable to find. + * @param value Output value if found. + * @return true if variable was found and parsed as integer. + */ +bool get_metric_value(const json& variables, const std::string& var_name, long& value) { + for (const auto& var : variables) { + // Try lowercase field names (new format) + if (var.contains("variable_name") && var["variable_name"] == var_name) { + if (var.contains("value")) { + try { + value = std::stol(var["value"].get()); + return true; + } catch (...) { + return false; + } + } + } + // Try uppercase field names (legacy format) + if (var.contains("Variable_Name") && var["Variable_Name"] == var_name) { + if (var.contains("Variable_Value")) { + try { + value = std::stol(var["Variable_Value"].get()); + return true; + } catch (...) { + return false; + } + } + } + } + return false; +} + } // namespace int main(int argc, char** argv) { @@ -166,88 +198,157 @@ int main(int argc, char** argv) { } bool mcp_reachable = false; + bool using_ssl = false; if (can_continue) { // Retry loop: MCP server may need a moment to start after LOAD MCP VARIABLES TO RUNTIME const int k_max_retries = 30; // 30 retries * 100ms = 3 seconds max wait const int k_retry_delay_ms = 100; int retry_count = 0; - while (!mcp_reachable && retry_count < k_max_retries) { - usleep(k_retry_delay_ms * 1000); - mcp_reachable = mcp->check_server(); - retry_count++; + // First try HTTP, then HTTPS if HTTP fails + for (int ssl_attempt = 0; ssl_attempt <= 1 && !mcp_reachable; ssl_attempt++) { + bool try_ssl = (ssl_attempt == 1); + mcp->set_use_ssl(try_ssl); + + if (try_ssl) { + diag("HTTP failed, trying HTTPS..."); + } + + retry_count = 0; + while (!mcp_reachable && retry_count < k_max_retries) { + usleep(k_retry_delay_ms * 1000); + mcp_reachable = mcp->check_server(); + retry_count++; + } + + if (mcp_reachable) { + using_ssl = try_ssl; + diag("MCP server reachable via %s after %d retries (%dms)", + try_ssl ? "HTTPS" : "HTTP", retry_count, retry_count * k_retry_delay_ms); + } } - if (mcp_reachable) { - diag("MCP server became reachable after %d retries (%dms)", retry_count, retry_count * k_retry_delay_ms); - } - - ok(mcp_reachable, "MCP server reachable at %s", mcp->get_connection_info().c_str()); + ok(mcp_reachable, "MCP server reachable at %s (%s)", mcp->get_connection_info().c_str(), + using_ssl ? "HTTPS" : "HTTP"); if (!mcp_reachable) { skip(7, "Cannot continue without MCP connectivity"); can_continue = false; } } - bool marker_deleted = false; - bool marker_inserted = false; - if (can_continue) { - // Inject synthetic stale row into stats global table. - marker_deleted = run_admin_stmt( - admin, - "DELETE FROM stats.stats_mysql_global WHERE Variable_Name='MCP_REFRESH_MARKER'", - "Delete stale marker row" - ); - marker_inserted = run_admin_stmt( - admin, - "INSERT OR REPLACE INTO stats.stats_mysql_global (Variable_Name, Variable_Value) VALUES ('" - + std::string(k_marker_name) + "', '" + std::string(k_marker_value) + "')", - "Insert stale marker row" - ); - ok(marker_deleted && marker_inserted, "Injected synthetic stale marker into stats.stats_mysql_global"); - if (!(marker_deleted && marker_inserted)) { - skip(6, "Cannot continue without marker row setup"); - can_continue = false; - } - } + // Variables needed across multiple blocks + json initial_vars = json::array(); + long initial_count = 0; + // Test: Query Client_Connections_connected, create connections, verify count increases if (can_continue) { - const MCPResponse marker_resp = mcp->call_tool( + diag("Step 1: Querying initial Client_Connections_connected via MCP stats endpoint"); + + // Step 1: Get initial connection count via MCP stats + const MCPResponse initial_resp = mcp->call_tool( "stats", "show_status", - json{{"db_type", "mysql"}, {"variable_name", k_marker_name}} + json{{"db_type", "mysql"}, {"variable_name", "Client_Connections_connected"}} ); - ok(marker_resp.is_success(), "MCP call stats.show_status(marker) transport/protocol success"); + ok(initial_resp.is_success(), "MCP call stats.show_status(Client_Connections_connected) transport success"); - json marker_vars = json::array(); - std::string marker_err; - const bool marker_payload_ok = extract_show_status_variables(marker_resp, marker_vars, marker_err); - ok(marker_payload_ok, "stats.show_status(marker) payload valid%s%s", - marker_payload_ok ? "" : ": ", marker_payload_ok ? "" : marker_err.c_str()); + // Debug: print raw response + diag("Raw HTTP response code: %ld", initial_resp.get_http_code()); + if (!initial_resp.is_success()) { + diag("Transport/protocol error: %s", initial_resp.get_error_message().c_str()); + } - const size_t marker_row_count = marker_payload_ok ? marker_vars.size() : 0; - ok(marker_payload_ok && marker_row_count == 0, - "Marker row removed after refresh-before-read (variables=%zu)", marker_row_count); + std::string initial_err; + bool initial_payload_ok = extract_show_status_variables(initial_resp, initial_vars, initial_err); + if (!initial_payload_ok) { + diag("Payload extraction failed: %s", initial_err.c_str()); + diag("Raw response body: %s", initial_resp.get_http_response().substr(0, 500).c_str()); + } + ok(initial_payload_ok, "Initial show_status payload valid%s%s", + initial_payload_ok ? "" : ": ", initial_payload_ok ? "" : initial_err.c_str()); + + if (initial_payload_ok) { + diag("Received %zu variables in response", initial_vars.size()); + bool found = get_metric_value(initial_vars, "Client_Connections_connected", initial_count); + if (found) { + diag("Client_Connections_connected initial value: %ld", initial_count); + } + ok(found, "Found Client_Connections_connected in initial response (value=%ld)", initial_count); + if (!found) { + skip(4, "Cannot continue without initial connection count"); + can_continue = false; + } + } else { + skip(5, "Cannot continue without valid initial payload"); + can_continue = false; + } + } + + // Create additional connections and verify count increases + if (can_continue) { + // Step 2: Create several new MySQL connections to the frontend port + const int k_new_connections = 5; + std::vector new_conns; + + diag("Step 2: Creating %d new MySQL connections to %s:%d to generate traffic", + k_new_connections, cl.host, cl.port); + for (int i = 0; i < k_new_connections; i++) { + MYSQL* conn = init_mysql_conn(cl.host, cl.port, cl.username, cl.password); + if (conn) { + new_conns.push_back(conn); + diag(" Created connection %d", i + 1); + } else { + diag(" Failed to create connection %d", i + 1); + } + } + diag("Successfully created %zu new connections", new_conns.size()); + ok(new_conns.size() >= 1, "Created at least 1 new connection (created %zu)", new_conns.size()); - const MCPResponse uptime_resp = mcp->call_tool( + // Step 3: Query connection count again + diag("Step 3: Querying Client_Connections_connected again via MCP stats endpoint"); + const MCPResponse updated_resp = mcp->call_tool( "stats", "show_status", - json{{"db_type", "mysql"}, {"variable_name", "ProxySQL_Uptime"}} + json{{"db_type", "mysql"}, {"variable_name", "Client_Connections_connected"}} ); - ok(uptime_resp.is_success(), "MCP call stats.show_status(ProxySQL_Uptime) transport/protocol success"); - - json uptime_vars = json::array(); - std::string uptime_err; - const bool uptime_payload_ok = extract_show_status_variables(uptime_resp, uptime_vars, uptime_err); - ok(uptime_payload_ok, "stats.show_status(ProxySQL_Uptime) payload valid%s%s", - uptime_payload_ok ? "" : ": ", uptime_payload_ok ? "" : uptime_err.c_str()); + ok(updated_resp.is_success(), "MCP call stats.show_status after connections transport success"); + + json updated_vars = json::array(); + std::string updated_err; + bool updated_payload_ok = extract_show_status_variables(updated_resp, updated_vars, updated_err); + if (!updated_payload_ok) { + diag("Payload extraction failed: %s", updated_err.c_str()); + diag("Raw response body: %s", updated_resp.get_http_response().substr(0, 500).c_str()); + } + ok(updated_payload_ok, "Updated show_status payload valid%s%s", + updated_payload_ok ? "" : ": ", updated_payload_ok ? "" : updated_err.c_str()); + + // Step 4: Verify count increased + if (updated_payload_ok) { + long updated_count = 0; + bool found = get_metric_value(updated_vars, "Client_Connections_connected", updated_count); + if (found) { + diag("Client_Connections_connected updated value: %ld (initial was: %ld)", updated_count, initial_count); + ok(updated_count > initial_count, + "Connection count increased after creating connections (before=%ld, after=%ld, diff=%ld)", + initial_count, updated_count, updated_count - initial_count); + } else { + diag("Client_Connections_connected not found in response"); + diag("Available variables: %s", updated_vars.dump().substr(0, 500).c_str()); + ok(false, "Client_Connections_connected not found in updated response"); + } + } else { + skip(1, "Cannot verify count without valid updated payload"); + } - ok(uptime_payload_ok && !uptime_vars.empty(), - "stats.show_status(ProxySQL_Uptime) returned at least one variable row"); + // Cleanup: close the connections we created + diag("Cleanup: Closing %zu test connections", new_conns.size()); + for (MYSQL* conn : new_conns) { + mysql_close(conn); + } } if (admin) { - run_q(admin, "DELETE FROM stats.stats_mysql_global WHERE Variable_Name='MCP_REFRESH_MARKER'"); run_q(admin, "SET mcp-stats_endpoint_auth=''"); run_q(admin, "SET mcp-enabled=false"); run_q(admin, "LOAD MCP VARIABLES TO RUNTIME");