diff --git a/.github/workflows/CI-mysqlx.yml b/.github/workflows/CI-mysqlx.yml index d4eaaa81c..ca04c5a01 100644 --- a/.github/workflows/CI-mysqlx.yml +++ b/.github/workflows/CI-mysqlx.yml @@ -20,6 +20,8 @@ jobs: uses: actions/checkout@v4 with: sparse-checkout: | + include + src/proxysql_global.cpp test/tap plugins/mysqlx test/scripts @@ -31,7 +33,10 @@ jobs: src/proxysql test/ key: build-${{ github.event.workflow_run && github.event.workflow_run.head_sha || github.sha }} - fail-on-cache-miss: true + # Strict cache dependency when CI-trigger populated the cache + # upstream; lenient on manual workflow_dispatch so a missing cache + # doesn't block an ad-hoc run. + fail-on-cache-miss: ${{ github.event_name == 'workflow_run' }} - name: Build mysqlx plugin run: | @@ -58,6 +63,8 @@ jobs: uses: actions/checkout@v4 with: sparse-checkout: | + include + src/proxysql_global.cpp test/tap plugins/mysqlx test/scripts @@ -69,7 +76,10 @@ jobs: src/proxysql test/ key: build-${{ github.event.workflow_run && github.event.workflow_run.head_sha || github.sha }} - fail-on-cache-miss: true + # Strict cache dependency when CI-trigger populated the cache + # upstream; lenient on manual workflow_dispatch so a missing cache + # doesn't block an ad-hoc run. + fail-on-cache-miss: ${{ github.event_name == 'workflow_run' }} - name: Build mysqlx plugin run: | diff --git a/doc/PLUGIN_API.md b/doc/PLUGIN_API.md index 2e586504b..7fe6d9ebd 100644 --- a/doc/PLUGIN_API.md +++ b/doc/PLUGIN_API.md @@ -41,16 +41,37 @@ startup phase before the database takes precedence). ### Startup Sequence -1. ProxySQL parses `proxysql.cnf` and populates the `plugins` list. -2. Admin module initializes (creates SQLite databases). -3. For each plugin path, ProxySQL calls `dlopen()` to load the `.so`. -4. ProxySQL resolves the `proxysql_plugin_descriptor_v1` symbol. -5. The plugin's `init()` callback is called, receiving `ProxySQL_PluginServices`. - The plugin registers its tables and commands during this call. -6. ProxySQL creates the registered SQLite tables. -7. The plugin's `start()` callback is called. The plugin should start its - threads, open listener sockets, and load runtime configuration. -8. ProxySQL is ready. The plugin is live. +ProxySQL uses a **four-phase** plugin lifecycle. Every phase but Phase B +is mandatory; Phase B is optional via the `register_schemas` descriptor +field and only enabled when the plugin declares ABI version 2. + +1. **Phase A — load.** ProxySQL parses `proxysql.cnf` and populates the + `plugins` list. For each plugin path, ProxySQL calls `dlopen()`, + resolves the `proxysql_plugin_descriptor_v1` symbol, and validates + the descriptor (`abi_version`, `name`, callback pointers). +2. **Phase B — register_schemas (optional, ABI 2 only).** If the + descriptor wires `register_schemas`, the loader invokes it with a + `ProxySQL_PluginServices` whose `register_table` / + `register_command` / `register_command_alias` entries are LIVE but + whose DB-handle getters (`get_admindb`, `get_configdb`, + `get_statsdb`) are non-null stubs that return `nullptr`. The plugin + declares the tables it owns and (optionally) its admin commands; it + MUST NOT touch DB handles here. Plugins that leave + `register_schemas` null (or that declare ABI 1) skip this phase + entirely and do all their setup in Phase D. +3. **Phase C — admin materialization.** The admin module initializes + and materializes the SQLite schemas collected during Phase B + (`merge_plugin_tables` + `CREATE TABLE`). On DDL failure ProxySQL + aborts startup. +4. **Phase D — init.** The plugin's `init()` callback is called, + receiving a fully live `ProxySQL_PluginServices` (DB handles now + valid). Plugins that opted out of Phase B register their tables + AND commands here; plugins that used Phase B only finish their + context setup. +5. **Phase E — start.** The plugin's `start()` callback is called. + The plugin should start its threads, open listener sockets, and + load runtime configuration. After this returns, ProxySQL is ready + and the plugin is live. ### Shutdown Sequence @@ -80,26 +101,37 @@ All types are defined in `include/ProxySQL_Plugin.h`: ```cpp struct ProxySQL_PluginDescriptor { const char *name; // Human-readable plugin name - uint32_t abi_version; // Must be 1 + uint32_t abi_version; // PROXYSQL_PLUGIN_ABI_VERSION (1 or 2) proxysql_plugin_init_cb init; // bool (*)(ProxySQL_PluginServices *) proxysql_plugin_start_cb start; // bool (*)() proxysql_plugin_stop_cb stop; // bool (*)() proxysql_plugin_status_json_cb status_json; // const char *(*)() + proxysql_plugin_register_schemas_cb register_schemas; // ABI 2 only, optional }; ``` -| Field | Type | Description | -|----------------|----------|-----------------------------------------------------------| -| `name` | `const char*` | Plugin identifier, used in logging. | -| `abi_version` | `uint32_t` | Must be `1`. | -| `init` | callback | Called once at startup. Receives `ProxySQL_PluginServices`. Register tables and commands here. | -| `start` | callback | Called after `init`. Start threads, open sockets, load config. | -| `stop` | callback | Called on shutdown. Stop threads, release resources. | -| `status_json` | callback | Return a JSON string describing plugin status. | +| Field | Type | Description | +|--------------------|---------------|-----------------------------------------------------------| +| `name` | `const char*` | Plugin identifier, used in logging. | +| `abi_version` | `uint32_t` | Set from `PROXYSQL_PLUGIN_ABI_VERSION`. Value `1` = pre-chassis descriptor (six fields). Value `2` = PROXYSQL40 descriptor (adds `register_schemas`). A v3/v3.1 ProxySQL core rejects `abi_version > 1`. | +| `init` | callback | Phase D — called with live services; register tables and commands here (or finish context setup if `register_schemas` already did it). | +| `start` | callback | Phase E — start threads, open sockets, load config. | +| `stop` | callback | Called on shutdown. Pairs with `init`, not `start`: if `init` returned true and `start` later failed, `stop` is still called so the plugin can release resources it allocated in `init`. | +| `status_json` | callback | Return a static JSON string describing plugin status. | +| `register_schemas` | callback | Phase B (ABI 2 only). Optional; leave null to skip Phase B entirely. Services passed here have `register_table` / `register_command` / `register_command_alias` LIVE but DB-handle getters returning `nullptr`. | All callbacks return `bool` (except `status_json` which returns `const char*`). -Return `true` on success, `false` on failure. A `false` return from `init` or -`start` causes ProxySQL to exit. +Return `true` on success, `false` on failure. A `false` return from +`register_schemas`, `init`, or `start` causes ProxySQL to exit. + +#### ABI version + +`include/ProxySQL_Plugin.h` exposes `PROXYSQL_PLUGIN_ABI_VERSION` (2 under +PROXYSQL40, undefined in pre-chassis builds — the descriptor is then a +legacy six-field struct with `abi_version = 1`). Plugins MUST assign +`abi_version` from this macro rather than hard-coding a literal; the +core's loader uses it to detect layout skew and reject plugins built +for an unsupported ABI. See `ProxySQL_Plugin.h` for the exact rules. ### The Entry Point diff --git a/lib/Admin_Bootstrap.cpp b/lib/Admin_Bootstrap.cpp index fdc259f2e..837351681 100644 --- a/lib/Admin_Bootstrap.cpp +++ b/lib/Admin_Bootstrap.cpp @@ -1352,7 +1352,17 @@ void ProxySQL_Admin::materialize_plugin_tables() { ProxySQL_PluginManager* plugin_manager = proxysql_get_plugin_manager(); if (!plugin_manager) return; - auto merge_new = [this](std::vector* target, const std::vector& defs) { + // merge_new returns the subset of `defs` that were actually added + // (skipping entries whose table_name already exists in `target`). + // The caller runs DDL for only this subset so a repeat invocation + // would not re-execute CREATE TABLE statements for tables that + // already exist in the SQLite database. + auto merge_new = [this]( + std::vector* target, + const std::vector& defs + ) -> std::vector { + std::vector added; + added.reserve(defs.size()); for (const auto& def : defs) { bool exists = false; for (const auto* existing : *target) { @@ -1363,23 +1373,34 @@ void ProxySQL_Admin::materialize_plugin_tables() { } if (!exists) { insert_into_tables_defs(target, def.table_name, def.table_def); + added.push_back(&def); } } + return added; }; - merge_new(tables_defs_admin, plugin_manager->tables(ProxySQL_PluginDBKind::admin_db)); - merge_new(tables_defs_config, plugin_manager->tables(ProxySQL_PluginDBKind::config_db)); - merge_new(tables_defs_stats, plugin_manager->tables(ProxySQL_PluginDBKind::stats_db)); - - for (const auto& def : plugin_manager->tables(ProxySQL_PluginDBKind::admin_db)) { - admindb->execute(def.table_def); - } - for (const auto& def : plugin_manager->tables(ProxySQL_PluginDBKind::config_db)) { - configdb->execute(def.table_def); - } - for (const auto& def : plugin_manager->tables(ProxySQL_PluginDBKind::stats_db)) { - statsdb->execute(def.table_def); - } + const auto new_admin = merge_new(tables_defs_admin, plugin_manager->tables(ProxySQL_PluginDBKind::admin_db)); + const auto new_config = merge_new(tables_defs_config, plugin_manager->tables(ProxySQL_PluginDBKind::config_db)); + const auto new_stats = merge_new(tables_defs_stats, plugin_manager->tables(ProxySQL_PluginDBKind::stats_db)); + + // Plugin-declared DDL failures are startup-fatal: a partially + // materialized schema would only surface later as opaque runtime + // errors from the plugin itself. Log the plugin, table name, and + // SQLite message, then abort. + auto run_ddl = [](SQLite3DB* db, const std::vector& defs, const char* kind) { + for (const auto* def : defs) { + if (!db->execute(def->table_def)) { + proxy_error("Plugin %s-db DDL failed for table %s: %s\n", + kind, + def->table_name ? def->table_name : "(null)", + def->table_def ? def->table_def : "(null)"); + exit(EXIT_FAILURE); + } + } + }; + run_ddl(admindb, new_admin, "admin"); + run_ddl(configdb, new_config, "config"); + run_ddl(statsdb, new_stats, "stats"); // admindb<->configdb("disk") and admindb<->statsdb("stats") are // already attached by init_sqlite3db() above this function in the diff --git a/plugins/mysqlx/Makefile b/plugins/mysqlx/Makefile index d876eb217..c7b9dccaa 100644 --- a/plugins/mysqlx/Makefile +++ b/plugins/mysqlx/Makefile @@ -1,6 +1,14 @@ #!/bin/make -f -PROXYSQL_PATH := $(shell while [ ! -f ./src/proxysql_global.cpp ]; do cd ..; done; pwd) +# Walk upward until we find the repo root (src/proxysql_global.cpp). A hop +# counter stops the climb at / on a sparse checkout that omits the marker +# file -- otherwise we'd loop forever. Fails fast with a clear message so +# build output doesn't look like it's hanging. +PROXYSQL_PATH := $(shell set -e; d="$$PWD"; for _ in 1 2 3 4 5 6 7 8 9 10 11 12; do if [ -f "$$d/src/proxysql_global.cpp" ]; then echo "$$d"; exit 0; fi; if [ "$$d" = "/" ]; then break; fi; d="$$(dirname "$$d")"; done; echo "ERROR: plugins/mysqlx/Makefile: could not find src/proxysql_global.cpp (repo root) from $$PWD; sparse checkout missing include/ and src/proxysql_global.cpp?" >&2; exit 1) + +ifneq (,$(findstring ERROR,$(PROXYSQL_PATH))) +$(error $(PROXYSQL_PATH)) +endif include $(PROXYSQL_PATH)/include/makefiles_vars.mk include $(PROXYSQL_PATH)/include/makefiles_paths.mk diff --git a/plugins/mysqlx/include/mysqlx_session.h b/plugins/mysqlx/include/mysqlx_session.h index 720b9c218..d5173b74f 100644 --- a/plugins/mysqlx/include/mysqlx_session.h +++ b/plugins/mysqlx/include/mysqlx_session.h @@ -12,10 +12,12 @@ class Mysqlx_Thread; struct MysqlxCredentials { - std::string password_hash; - bool x_enabled; - std::string allowed_auth; - std::string backend_password; + std::string password_hash {}; + // Default to false so default-constructed (e.g. lookup-miss) values + // never authenticate by accident via an indeterminate bool. + bool x_enabled { false }; + std::string allowed_auth {}; + std::string backend_password {}; }; typedef std::function MysqlxCredentialLookup; diff --git a/plugins/mysqlx/src/mysqlx_backend_session.cpp b/plugins/mysqlx/src/mysqlx_backend_session.cpp index 89c7b258f..af4132903 100644 --- a/plugins/mysqlx/src/mysqlx_backend_session.cpp +++ b/plugins/mysqlx/src/mysqlx_backend_session.cpp @@ -83,7 +83,15 @@ bool MysqlxBackendSession::connect(const MysqlxResolvedIdentity& identity, std::string backend_user = identity.backend_username; std::string backend_pass = identity.backend_password; - return authenticate_backend(backend_user, backend_pass, err); + if (!authenticate_backend(backend_user, backend_pass, err)) { + // Auth failure: close the socket we just opened so a later retry + // on this MysqlxBackendSession doesn't overwrite backend_fd_ and + // leak the previous fd. + close(backend_fd_); + backend_fd_ = -1; + return false; + } + return true; } bool MysqlxBackendSession::authenticate_backend(const std::string& username,