Merge branch '2.3' into develop

Documentation/Authenticators/PAM-Authenticator.md

@@ -51,21 +51,29 @@ account         required        pam_unix.so
 
 ## Anonymous user mapping
 
-The MaxScale PAM authenticator supports a limited version of
+The MaxScale PAM authenticator supports a limited version of [user
-[user mapping](https://mariadb.com/kb/en/library/user-and-group-mapping-with-pam/).
+mapping](https://mariadb.com/kb/en/library/user-and-group-mapping-with-pam/). It requires
-Anonymous mapping is enabled in MaxScale if the following user exists:
+less configuration but is also less accurate than the server authentication. Anonymous
-- Empty username and wildcard host (`''@'%'`)
+mapping is enabled in MaxScale if the following user exists:
+- Empty username (e.g. `''@'%'` or `''@'myhost.com'`)
 - `plugin = 'pam'`
-- Proxy grant is on (The query `SHOW GRANTS FOR ''@'%';` returns `GRANT PROXY ON ...`
+- Proxy grant is on (The query `SHOW GRANTS FOR user@host;` returns at least one row with
+  `GRANT PROXY ON ...`)
 
-When the authenticator detects such a user, anonymous account mapping is enabled.
+When the authenticator detects such users, anonymous account mapping is enabled for the
-To verify this, search the MaxScale log for "Anonymous PAM user with proxy grant
+hosts of the anonymous users. To verify this, enable the info log (`log_info=1` in
-found. User account mapping enabled." When mapping is on, the PAM authenticator
+MaxScale config file) and look for messages such as "Found 2 anonymous PAM user(s) ..."
-does not require client accounts to exist in the `mysql.user`-table received from
+and "Added anonymous PAM user ..." during MaxScale startup.
-the backend. It will simply authenticate the client to the local machine with
+
-the username and password supplied. The PAM service used for authentication is
+When mapping is on, the MaxScale PAM authenticator does not require client accounts to
-read from the `authentication_string`-field of the anonymous user. If authentication
+exist in the `mysql.user`-table received from the backend. MaxScale only requires that the
-was successful, MaxScale then uses the username and password to log to the backends.
+hostname of the incoming client matches the host field of one of the anonymous users
+(comparison performed using `LIKE`). If a match is found, MaxScale attempts to
+authenticate the client to the local machine with the username and password supplied. The
+PAM service used for authentication is read from the `authentication_string`-field of the
+anonymous user. If authentication was successful, MaxScale then uses the username and
+password to log to the backends. If the client host matches multiple anonymous hosts,
+authentication is attempted with all of their PAM services until one succeeds or all fail.
 
 Anonymous mapping is only attempted if the client username is not found in the
 `mysql.user`-table as explained in [Configuration](#configuration). This means,
@@ -73,6 +81,15 @@ that if a user is found and the authentication fails, anonymous authentication
 is not attempted even when it could use a different PAM service with a different
 outcome.
 
+Setting up PAM group mapping for the MariaDB server is a more involved process as the
+server requires details on which Unix user or group is mapped to which MariaDB user. See
+[this guide](https://mariadb.com/kb/en/library/configuring-pam-authentication-and-user-mapping-with-unix-authentication/)
+for more details. Performing all the steps in the guide also on the MaxScale machine is
+not required, as the MaxScale PAM plugin only checks that the client host matches an
+anonymous user and that the client (with the username and password it provided) can log
+into the local PAM configuration. If using normal password authentication, simply
+generating the Unix user and password should be enough.
+
 ## Implementation details and limitations
 
 The PAM general authentication scheme is difficult for a proxy such as MaxScale.

maxscale-system-test/mariadb_nodes.cpp

@@ -395,8 +395,7 @@ void Mariadb_nodes::create_users(int node)
     char dtr[PATH_MAX + 1024];
     // Create users for replication as well as the users that are used by the tests
     sprintf(str, "%s/create_user.sh", test_dir);
-    sprintf(dtr, "%s", access_homedir[node]);
+    copy_to_node(node, str, access_homedir[node]);
-    copy_to_node(node, str, dtr);
     ssh_node_f(node, false,
                "export node_user=\"%s\"; export node_password=\"%s\"; %s/create_user.sh %s",
                user_name, password, access_homedir[0], socket_cmd[0]);

server/modules/monitor/mariadbmon/cluster_manipulation.cc

@@ -234,6 +234,9 @@ bool MariaDBMonitor::manual_reset_replication(SERVER* master_server, json_t** er
         }
     }
 
+    // Also record the previous master, needed for scheduled events.
+    MariaDBServer* old_master = (m_master && m_master->is_master()) ? m_master : nullptr;
+
     bool rval = false;
     if (new_master)
     {
@@ -333,13 +336,21 @@ bool MariaDBMonitor::manual_reset_replication(SERVER* master_server, json_t** er
 
                 if (m_handle_event_scheduler)
                 {
-                    if (!new_master->enable_events(error_out))
+                    if (old_master)
+                    {
+                        if (!new_master->enable_events(old_master->m_enabled_events, error_out))
                         {
                             error = true;
                             PRINT_MXS_JSON_ERROR(error_out, "Could not enable events on '%s': %s",
                                                  new_master->name(), error_msg.c_str());
                         }
                     }
+                    else
+                    {
+                        MXS_WARNING("No scheduled events were enabled on '%s' because previous master is "
+                                    "unknown. Check events manually.", new_master->name());
+                    }
+                }
 
                 // Add an event to the new master so that it has a non-empty gtid_current_pos.
                 if (!new_master->execute_cmd("FLUSH TABLES;", &error_msg))
@@ -626,7 +637,7 @@ uint32_t MariaDBMonitor::do_rejoin(const ServerArray& joinable_servers, json_t**
                 // Assume that server is an old master which was failed over. Even if this is not really
                 // the case, the following is unlikely to do damage.
                 ServerOperation demotion(joinable, true,  /* treat as old master */
-                                         m_handle_event_scheduler, m_demote_sql_file, {} /* unused */);
+                                         m_handle_event_scheduler, m_demote_sql_file);
                 if (joinable->demote(general, demotion))
                 {
                     MXS_NOTICE("Directing standalone server '%s' to replicate from '%s'.", name, master_name);
@@ -1398,7 +1409,7 @@ unique_ptr<MariaDBMonitor::FailoverParams> MariaDBMonitor::failover_prepare(Log
             bool promoting_to_master = (demotion_target == m_master);
             ServerOperation promotion(promotion_target, promoting_to_master,
                                       m_handle_event_scheduler, m_promote_sql_file,
-                                         demotion_target->m_slave_status);
+                                      demotion_target->m_slave_status, demotion_target->m_enabled_events);
             GeneralOpData general(m_replication_user, m_replication_password, error_out, time_limit);
             rval.reset(new FailoverParams(promotion, demotion_target, general));
         }
@@ -1687,9 +1698,10 @@ MariaDBMonitor::switchover_prepare(SERVER* promotion_server, SERVER* demotion_se
         maxbase::Duration time_limit((double)m_switchover_timeout);
         bool master_swap = (demotion_target == m_master);
         ServerOperation promotion(promotion_target, master_swap, m_handle_event_scheduler,
-                                  m_promote_sql_file, demotion_target->m_slave_status);
+                                  m_promote_sql_file,
+                                  demotion_target->m_slave_status, demotion_target->m_enabled_events);
         ServerOperation demotion(demotion_target, master_swap, m_handle_event_scheduler,
-                                 m_demote_sql_file, promotion_target->m_slave_status);
+                                 m_demote_sql_file, promotion_target->m_slave_status, {} /* unused */);
         GeneralOpData general(m_replication_user, m_replication_password, error_out, time_limit);
         rval.reset(new SwitchoverParams(promotion, demotion, general));
     }

server/modules/monitor/mariadbmon/mariadbmon.cc

@@ -80,7 +80,8 @@ void MariaDBMonitor::reset_server_info()
     // Next, initialize the data.
     for (auto mon_server : Monitor::m_servers)
     {
-        m_servers.push_back(new MariaDBServer(mon_server, m_servers.size(), m_assume_unique_hostnames));
+        m_servers.push_back(new MariaDBServer(mon_server, m_servers.size(),
+                            m_assume_unique_hostnames, m_handle_event_scheduler));
     }
 }
 

server/modules/monitor/mariadbmon/mariadbserver.cc

@@ -28,20 +28,12 @@ using maxbase::Duration;
 using maxbase::StopWatch;
 using maxsql::QueryResult;
 
-class MariaDBServer::EventInfo
-{
-public:
-    std::string database;
-    std::string name;
-    std::string definer;
-    std::string status;
-};
-
 MariaDBServer::MariaDBServer(MXS_MONITORED_SERVER* monitored_server, int config_index,
-                             bool assume_unique_hostnames)
+                             bool assume_unique_hostnames, bool query_events)
     : m_server_base(monitored_server)
     , m_config_index(config_index)
     , m_assume_unique_hostnames(assume_unique_hostnames)
+    , m_query_events(query_events)
 {
     mxb_assert(monitored_server);
 }
@@ -861,6 +853,10 @@ void MariaDBServer::monitor_server()
         {
             query_ok = update_gtids(&errmsg);
         }
+        if (query_ok && m_query_events)
+        {
+            query_ok = update_enabled_events();
+        }
     }
     else
     {
@@ -1245,14 +1241,16 @@ const SlaveStatus* MariaDBServer::slave_connection_status_host_port(const MariaD
     return NULL;
 }
 
-bool MariaDBServer::enable_events(json_t** error_out)
+bool MariaDBServer::enable_events(const EventNameSet& event_names, json_t** error_out)
 {
     int found_disabled_events = 0;
     int events_enabled = 0;
-    // Helper function which enables a slaveside disabled event.
+
-    ManipulatorFunc enabler = [this, &found_disabled_events, &events_enabled](const EventInfo& event,
+    // Helper function which enables a disabled event if that event name is found in the events-set.
-                                                                              json_t** error_out) {
+    ManipulatorFunc enabler = [this, event_names, &found_disabled_events, &events_enabled](
-            if (event.status == "SLAVESIDE_DISABLED")
+            const EventInfo& event, json_t** error_out) {
+        if (event_names.count(event.name) > 0
+            && (event.status == "SLAVESIDE_DISABLED" || event.status == "DISABLED"))
         {
             found_disabled_events++;
             if (alter_event(event, "ENABLE", error_out))
@@ -1384,8 +1382,7 @@ bool MariaDBServer::events_foreach(ManipulatorFunc& func, json_t** error_out)
     while (event_info->next_row())
     {
         EventInfo event;
-        event.database = event_info->get_string(db_name_ind);
+        event.name = event_info->get_string(db_name_ind) + "." + event_info->get_string(event_name_ind);
-        event.name = event_info->get_string(event_name_ind);
         event.definer = event_info->get_string(event_definer_ind);
         event.status = event_info->get_string(event_status_ind);
         func(event, error_out);
@@ -1405,10 +1402,6 @@ bool MariaDBServer::alter_event(const EventInfo& event, const string& target_sta
 {
     bool rval = false;
     string error_msg;
-    // First switch to the correct database.
-    string use_db_query = string_printf("USE %s;", event.database.c_str());
-    if (execute_cmd(use_db_query, &error_msg))
-    {
     // An ALTER EVENT by default changes the definer (owner) of the event to the monitor user.
     // This causes problems if the monitor user does not have privileges to run
     // the event contents. Prevent this by setting definer explicitly.
@@ -1428,6 +1421,7 @@ bool MariaDBServer::alter_event(const EventInfo& event, const string& target_sta
         // Just the username
         quoted_definer = event.definer;
     }
+
     string alter_event_query = string_printf("ALTER DEFINER = %s EVENT %s %s;",
                                              quoted_definer.c_str(),
                                              event.name.c_str(),
@@ -1435,21 +1429,13 @@ bool MariaDBServer::alter_event(const EventInfo& event, const string& target_sta
     if (execute_cmd(alter_event_query, &error_msg))
     {
         rval = true;
-            const char FMT[] = "Event '%s' of database '%s' on server '%s' set to '%s'.";
+        const char FMT[] = "Event '%s' on server '%s' set to '%s'.";
-            MXS_NOTICE(FMT, event.name.c_str(), event.database.c_str(), name(), target_status.c_str());
+        MXS_NOTICE(FMT, event.name.c_str(), name(), target_status.c_str());
     }
     else
     {
-            const char FMT[] = "Could not alter event '%s' of database '%s' on server '%s': %s";
+        const char FMT[] = "Could not alter event '%s' on server '%s': %s";
-            PRINT_MXS_JSON_ERROR(error_out, FMT, event.name.c_str(), event.database.c_str(), name(),
+        PRINT_MXS_JSON_ERROR(error_out, FMT, event.name.c_str(), name(), error_msg.c_str());
-                                 error_msg.c_str());
-        }
-    }
-    else
-    {
-        const char FMT[] = "Could not switch to database '%s' on '%s': %s Event '%s' not altered.";
-        PRINT_MXS_JSON_ERROR(error_out, FMT, event.database.c_str(), name(), error_msg.c_str(),
-                             event.name.c_str());
     }
     return rval;
 }
@@ -1533,7 +1519,7 @@ bool MariaDBServer::promote(GeneralOpData& general, ServerOperation& promotion,
                 if (promotion.handle_events)
                 {
                     // TODO: Add query replying to enable_events
-                    bool events_enabled = enable_events(error_out);
+                    bool events_enabled = enable_events(promotion.events_to_enable, error_out);
                     general.time_remaining -= timer.restart();
                     if (!events_enabled)
                     {
@@ -2142,3 +2128,33 @@ bool MariaDBServer::redirect_existing_slave_conn(GeneralOpData& op, const SlaveS
     }   // 'stop_slave_conn' prints its own errors
     return success;
 }
+
+bool MariaDBServer::update_enabled_events()
+{
+    string error_msg;
+    // Get names of all enabled scheduled events on the server.
+    auto event_info = execute_query("SELECT Event_schema, Event_name FROM information_schema.EVENTS WHERE "
+                                    "Status = 'ENABLED';", &error_msg);
+    if (event_info.get() == NULL)
+    {
+                MXS_ERROR("Could not query events of '%s': %s Event handling can be disabled by "
+                          "setting '%s' to false.",
+                          name(), error_msg.c_str(), CN_HANDLE_EVENTS);
+        return false;
+    }
+
+    auto db_name_ind = 0;
+    auto event_name_ind = 1;
+
+    EventNameSet full_names;
+    full_names.reserve(event_info->get_row_count());
+
+    while (event_info->next_row())
+    {
+        string full_name = event_info->get_string(db_name_ind) + "." + event_info->get_string(event_name_ind);
+        full_names.insert(full_name); // Ignore duplicates, they shouldn't exists.
+    }
+
+    m_enabled_events = std::move(full_names);
+    return true;
+}

server/modules/monitor/mariadbmon/mariadbserver.hh

@@ -72,6 +72,17 @@ struct NodeData
 class MariaDBServer
 {
 public:
+    MariaDBServer(MXS_MONITORED_SERVER* monitored_server, int config_index,
+                  bool assume_unique_hostnames, bool query_events);
+
+    class EventInfo
+    {
+    public:
+        std::string name;       /**< Event name in <database.name> form */
+        std::string definer;    /**< Definer of the event */
+        std::string status;     /**< Status of the event */
+    };
+
     enum class server_type
     {
         UNKNOWN,            /* Totally unknown. Server has not been connected to yet. */
@@ -136,10 +147,10 @@ public:
      * 'update_replication_settings' before use. */
     ReplicationSettings m_rpl_settings;
 
-    bool m_print_update_errormsg = true;    /* Should an update error be printed? */
+    bool         m_query_events; /* Copy of monitor->m_handle_event_scheduler. TODO: move elsewhere */
+    EventNameSet m_enabled_events; /* Enabled scheduled events */
 
-    MariaDBServer(MXS_MONITORED_SERVER* monitored_server, int config_index,
+    bool m_print_update_errormsg = true;    /* Should an update error be printed? */
-                  bool assume_unique_hostnames = true);
 
     /**
      * Print server information to a json object.
@@ -330,12 +341,14 @@ public:
     bool run_sql_from_file(const std::string& path, json_t** error_out);
 
     /**
-     * Enable any "SLAVESIDE_DISABLED" events. Event scheduler is not touched.
+     * Enable any "SLAVESIDE_DISABLED" or "DISABLED events. Event scheduler is not touched. Only events
+     * with names matching an element in the event_names set are enabled.
      *
+     * @param event_names Names of events that should be enabled
      * @param error_out Error output
      * @return True if all SLAVESIDE_DISABLED events were enabled
      */
-    bool enable_events(json_t** error_out);
+    bool enable_events(const EventNameSet& event_names, json_t** error_out);
 
     /**
      * Disable any "ENABLED" events. Event scheduler is not touched.
@@ -517,7 +530,6 @@ public:
     void set_status(uint64_t bits);
 
 private:
-    class EventInfo;
     typedef std::function<void (const EventInfo&, json_t** error_out)> ManipulatorFunc;
 
     enum class StopMode
@@ -559,4 +571,6 @@ private:
     bool        set_read_only(ReadOnlySetting value, maxbase::Duration time_limit, json_t** error_out);
     bool        merge_slave_conns(GeneralOpData& op, const SlaveStatusArray& conns_to_merge);
     std::string generate_change_master_cmd(GeneralOpData& op, const SlaveStatus& slave_conn);
+
+    bool update_enabled_events();
 };

server/modules/monitor/mariadbmon/server_utils.cc

@@ -155,14 +155,21 @@ bool SlaveStatus::should_be_copied(string* ignore_reason_out) const
     return accepted;
 }
 
-ServerOperation::ServerOperation(MariaDBServer* target, bool was_is_master,
+ServerOperation::ServerOperation(MariaDBServer* target, bool was_is_master, bool handle_events,
-                                 bool handle_events, const std::string& sql_file,
+                                 const std::string& sql_file, const SlaveStatusArray& conns_to_copy,
-                                 const SlaveStatusArray& conns_to_copy)
+                                 const EventNameSet& events_to_enable)
     : target(target)
     , to_from_master(was_is_master)
     , handle_events(handle_events)
     , sql_file(sql_file)
     , conns_to_copy(conns_to_copy)
+    , events_to_enable(events_to_enable)
+{
+}
+
+ServerOperation::ServerOperation(MariaDBServer* target, bool was_is_master, bool handle_events,
+                                 const std::string& sql_file)
+    : ServerOperation(target, was_is_master, handle_events, sql_file, {}, {})
 {
 }
 

server/modules/monitor/mariadbmon/server_utils.hh

@@ -13,6 +13,7 @@
 #pragma once
 #include "mariadbmon_common.hh"
 
+#include <unordered_set>
 #include <string>
 #include <vector>
 #include <maxbase/stopwatch.hh>
@@ -207,7 +208,8 @@ public:
     bool                      should_be_copied(std::string* ignore_reason_out) const;
 };
 
-typedef std::vector<SlaveStatus> SlaveStatusArray;
+using SlaveStatusArray = std::vector<SlaveStatus>;
+using EventNameSet = std::unordered_set<std::string>;
 
 enum class OperationType
 {
@@ -237,6 +239,13 @@ public:
     const std::string      sql_file;        // Path to file with SQL commands to run during op
     const SlaveStatusArray conns_to_copy;   // Slave connections the target should copy/merge
 
+    const EventNameSet events_to_enable; // Scheduled event names last seen on master.
+
     ServerOperation(MariaDBServer* target, bool was_is_master, bool handle_events,
-                    const std::string& sql_file, const SlaveStatusArray& conns_to_copy);
+                    const std::string& sql_file, const SlaveStatusArray& conns_to_copy,
+                    const EventNameSet& events_to_enable);
+
+    ServerOperation(MariaDBServer* target, bool was_is_master, bool handle_events,
+                    const std::string& sql_file);
+
 };

server/modules/monitor/mariadbmon/test/test_cycle_find.cc

@@ -159,7 +159,7 @@ void MariaDBMonitor::Test::init_servers(int count)
         // Server contents mostly undefined
         auto base_server = Server::create_test_server();
         MXS_MONITORED_SERVER* mon_server = new MXS_MONITORED_SERVER(base_server);
-        MariaDBServer* mariadb_server = new MariaDBServer(mon_server, i - 1, m_use_hostnames);
+        MariaDBServer* mariadb_server = new MariaDBServer(mon_server, i - 1, m_use_hostnames, true);
 
         if (m_use_hostnames)
         {