diff --git a/include/ProxySQL_PluginManager.h b/include/ProxySQL_PluginManager.h index 68bff61a5..4dd6057bf 100644 --- a/include/ProxySQL_PluginManager.h +++ b/include/ProxySQL_PluginManager.h @@ -27,7 +27,7 @@ public: void register_table_for_test(const ProxySQL_PluginTableDef& def); bool register_command_for_test(const std::string& sql); bool has_command_for_test(const std::string& sql) const; - void register_table(const ProxySQL_PluginTableDef& def); + bool register_table(const ProxySQL_PluginTableDef& def); bool register_command(const char* sql, proxysql_plugin_admin_command_cb cb); size_t size() const; diff --git a/lib/Admin_Bootstrap.cpp b/lib/Admin_Bootstrap.cpp index 4f180a938..1b60314d5 100644 --- a/lib/Admin_Bootstrap.cpp +++ b/lib/Admin_Bootstrap.cpp @@ -939,15 +939,27 @@ bool ProxySQL_Admin::init(const bootstrap_info_t& bootstrap_info) { #endif /* PROXYSQLGENAI */ if (ProxySQL_PluginManager* plugin_manager = proxysql_get_plugin_manager()) { - for (const auto& def : plugin_manager->tables(ProxySQL_PluginDBKind::admin_db)) { - insert_into_tables_defs(tables_defs_admin, def.table_name, def.table_def); - } - for (const auto& def : plugin_manager->tables(ProxySQL_PluginDBKind::config_db)) { - insert_into_tables_defs(tables_defs_config, def.table_name, def.table_def); - } - for (const auto& def : plugin_manager->tables(ProxySQL_PluginDBKind::stats_db)) { - insert_into_tables_defs(tables_defs_stats, def.table_name, def.table_def); - } + auto merge_plugin_tables = [this](std::vector* target, const std::vector& defs, const char* db_name) { + for (const auto& def : defs) { + bool duplicate_name = false; + for (const auto* existing : *target) { + if (strcasecmp(existing->table_name, def.table_name) == 0) { + duplicate_name = true; + break; + } + } + if (duplicate_name) { + proxy_warning("Skipping plugin table %s for %s because the table name already exists\n", + def.table_name, db_name); + continue; + } + insert_into_tables_defs(target, def.table_name, def.table_def); + } + }; + + merge_plugin_tables(tables_defs_admin, plugin_manager->tables(ProxySQL_PluginDBKind::admin_db), "admin"); + merge_plugin_tables(tables_defs_config, plugin_manager->tables(ProxySQL_PluginDBKind::config_db), "config"); + merge_plugin_tables(tables_defs_stats, plugin_manager->tables(ProxySQL_PluginDBKind::stats_db), "stats"); } // init ldap here diff --git a/lib/ProxySQL_PluginManager.cpp b/lib/ProxySQL_PluginManager.cpp index 918e2fae4..55cea552b 100644 --- a/lib/ProxySQL_PluginManager.cpp +++ b/lib/ProxySQL_PluginManager.cpp @@ -17,6 +17,10 @@ constexpr char kPluginCommandPrefix[] = "PLUGIN "; ProxySQL_PluginManager* g_active_plugin_manager = nullptr; ProxySQL_PluginManager* g_registry_target = nullptr; +ProxySQL_PluginCommandResult ignored_test_command(const ProxySQL_PluginCommandContext&, const char*) { + return {0, 0, ""}; +} + std::string format_dl_error(const char *prefix) { const char *dl_err = dlerror(); if (dl_err == nullptr) { @@ -33,14 +37,16 @@ std::string plugin_name(const ProxySQL_PluginDescriptor *descriptor) { } void register_table_service(const ProxySQL_PluginTableDef& def) { - if (g_registry_target != nullptr) { - g_registry_target->register_table(def); + if (g_registry_target != nullptr && !g_registry_target->register_table(def)) { + proxy_warning("Plugin table registration failed for %s\n", + def.table_name != nullptr ? def.table_name : "(null)"); } } void register_command_service(const char* sql, proxysql_plugin_admin_command_cb cb) { - if (g_registry_target != nullptr) { - g_registry_target->register_command(sql, cb); + if (g_registry_target != nullptr && !g_registry_target->register_command(sql, cb)) { + proxy_warning("Plugin command registration failed for %s\n", + sql != nullptr ? sql : "(null)"); } } @@ -258,7 +264,7 @@ void ProxySQL_PluginManager::register_table_for_test(const ProxySQL_PluginTableD } bool ProxySQL_PluginManager::register_command_for_test(const std::string& sql) { - return register_command(sql.c_str(), nullptr); + return register_command(sql.c_str(), &ignored_test_command); } bool ProxySQL_PluginManager::has_command_for_test(const std::string& sql) const { @@ -270,11 +276,29 @@ bool ProxySQL_PluginManager::has_command_for_test(const std::string& sql) const return false; } -void ProxySQL_PluginManager::register_table(const ProxySQL_PluginTableDef& def) { +bool ProxySQL_PluginManager::register_table(const ProxySQL_PluginTableDef& def) { if (def.table_name == nullptr || *def.table_name == '\0' || def.table_def == nullptr || *def.table_def == '\0') { - proxy_warning("Ignoring plugin table registration with empty table metadata\n"); - return; + return false; + } + + const std::vector* existing_tables = nullptr; + switch (def.db_kind) { + case ProxySQL_PluginDBKind::admin_db: + existing_tables = &tables_admin_; + break; + case ProxySQL_PluginDBKind::config_db: + existing_tables = &tables_config_; + break; + case ProxySQL_PluginDBKind::stats_db: + existing_tables = &tables_stats_; + break; + } + + for (const auto& existing : *existing_tables) { + if (strcasecmp(existing.table_name, def.table_name) == 0) { + return false; + } } table_storage_.push_back({def.table_name, def.table_def}); @@ -296,10 +320,12 @@ void ProxySQL_PluginManager::register_table(const ProxySQL_PluginTableDef& def) tables_stats_.push_back(owned_def); break; } + + return true; } bool ProxySQL_PluginManager::register_command(const char* sql, proxysql_plugin_admin_command_cb cb) { - if (sql == nullptr || *sql == '\0' || !has_plugin_command_prefix(sql)) { + if (sql == nullptr || *sql == '\0' || !has_plugin_command_prefix(sql) || cb == nullptr) { return false; } diff --git a/test/tap/tests/unit/plugin_registry_unit-t.cpp b/test/tap/tests/unit/plugin_registry_unit-t.cpp index 8a837ea00..665d53fa0 100644 --- a/test/tap/tests/unit/plugin_registry_unit-t.cpp +++ b/test/tap/tests/unit/plugin_registry_unit-t.cpp @@ -13,16 +13,23 @@ ProxySQL_PluginCommandResult fake_plugin_command(const ProxySQL_PluginCommandCon } // namespace int main() { - plan(9); + plan(10); ProxySQL_PluginManager mgr; char table_name[] = "mysqlx_users"; char table_def[] = "CREATE TABLE mysqlx_users (username VARCHAR NOT NULL PRIMARY KEY)"; + char duplicate_table_name[] = "mysqlx_users"; + char duplicate_table_def[] = "CREATE TABLE mysqlx_users (username VARCHAR NOT NULL PRIMARY KEY)"; ProxySQL_PluginTableDef def { ProxySQL_PluginDBKind::admin_db, table_name, table_def }; + ProxySQL_PluginTableDef duplicate_def { + ProxySQL_PluginDBKind::admin_db, + duplicate_table_name, + duplicate_table_def + }; mgr.register_table_for_test(def); table_name[0] = 'X'; @@ -31,6 +38,8 @@ int main() { ok(mgr.tables(ProxySQL_PluginDBKind::admin_db).size() == static_cast(1), "plugin admin table is stored"); ok(std::strcmp(mgr.tables(ProxySQL_PluginDBKind::admin_db).front().table_name, "mysqlx_users") == 0, "plugin admin table name is copied"); ok(std::strcmp(mgr.tables(ProxySQL_PluginDBKind::admin_db).front().table_def, "CREATE TABLE mysqlx_users (username VARCHAR NOT NULL PRIMARY KEY)") == 0, "plugin admin table definition is copied"); + mgr.register_table_for_test(duplicate_def); + ok(mgr.tables(ProxySQL_PluginDBKind::admin_db).size() == static_cast(1), "duplicate table registration is rejected"); ok(mgr.tables(ProxySQL_PluginDBKind::config_db).size() == static_cast(0), "config tables start empty"); ok(!mgr.register_command_for_test("SELECT 1"), "unnamespaced admin SQL is rejected"); ok(mgr.register_command("PLUGIN MYSQLX LOAD USERS TO RUNTIME", &fake_plugin_command), "namespaced command registration succeeds");