diff --git a/include/maxscale/monitor.hh b/include/maxscale/monitor.hh index cd9cb59a4..dbb695b70 100644 --- a/include/maxscale/monitor.hh +++ b/include/maxscale/monitor.hh @@ -244,6 +244,16 @@ public: uint64_t events; /**< Enabled monitor events. */ protected: + + /** + * Check if the monitor user can execute a query. The query should be such that it only succeeds if + * the monitor user has all required permissions. Servers which are down are skipped. + * + * @param query Query to test with + * @return True on success, false if monitor credentials lack permissions + */ + bool test_permissions(const std::string& query); + /** * Contains monitor base class settings. Since monitors are stopped before a setting change, * the items cannot be modified while a monitor is running. No locking required. @@ -286,8 +296,6 @@ extern const char CN_MONITOR_INTERVAL[]; extern const char CN_SCRIPT[]; extern const char CN_SCRIPT_TIMEOUT[]; -bool check_monitor_permissions(Monitor* monitor, const char* query); - /** * Store the current server status to the previous and pending status * fields of the monitored server. @@ -614,7 +622,7 @@ protected: * * @return True, if the monitor user has sufficient rights, false otherwise. */ - virtual bool has_sufficient_permissions() const; + virtual bool has_sufficient_permissions(); /** * @brief Flush pending server status to each server. diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 8e9684b14..d5b1c74d6 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -702,15 +702,9 @@ std::unique_ptr monitor_get_list() return set; } -/** - * @brief Check if the monitor user has all required permissions to operate properly. - * - * @param service Monitor to inspect - * @param query Query to execute - * @return True on success, false if monitor credentials lack permissions - */ -bool check_monitor_permissions(Monitor* monitor, const char* query) +bool Monitor::test_permissions(const string& query) { + auto monitor = this; if (monitor->monitored_servers == NULL // No servers to check || config_get_global_options()->skip_permission_checks) { @@ -719,7 +713,6 @@ bool check_monitor_permissions(Monitor* monitor, const char* query) char* user = monitor->user; char* dpasswd = decrypt_password(monitor->password); - MXS_CONFIG* cnf = config_get_global_options(); bool rval = false; for (MXS_MONITORED_SERVER* mondb = monitor->monitored_servers; mondb; mondb = mondb->next) @@ -745,7 +738,7 @@ bool check_monitor_permissions(Monitor* monitor, const char* query) break; } } - else if (mxs_mysql_query(mondb->con, query) != 0) + else if (mxs_mysql_query(mondb->con, query.c_str()) != 0) { switch (mysql_errno(mondb->con)) { @@ -763,10 +756,7 @@ bool check_monitor_permissions(Monitor* monitor, const char* query) } MXS_ERROR("[%s] Failed to execute query '%s' with user '%s'. MySQL error message: %s", - monitor->name, - query, - user, - mysql_error(mondb->con)); + monitor->name, query.c_str(), user, mysql_error(mondb->con)); } else { @@ -2669,7 +2659,7 @@ bool MonitorWorker::configure(const MXS_CONFIG_PARAMETER* pParams) return true; } -bool MonitorWorker::has_sufficient_permissions() const +bool MonitorWorker::has_sufficient_permissions() { return true; } diff --git a/server/modules/monitor/auroramon/auroramon.cc b/server/modules/monitor/auroramon/auroramon.cc index 2bcd797cd..1aa73ae41 100644 --- a/server/modules/monitor/auroramon/auroramon.cc +++ b/server/modules/monitor/auroramon/auroramon.cc @@ -40,12 +40,10 @@ AuroraMonitor* AuroraMonitor::create(const std::string& name, const std::string& return new AuroraMonitor(name, module); } -bool AuroraMonitor::has_sufficient_permissions() const +bool AuroraMonitor::has_sufficient_permissions() { - return check_monitor_permissions(m_monitor, - "SELECT @@aurora_server_id, server_id FROM " - "information_schema.replica_host_status " - "WHERE session_id = 'MASTER_SESSION_ID'"); + return test_permissions("SELECT @@aurora_server_id, server_id FROM " + "information_schema.replica_host_status WHERE session_id = 'MASTER_SESSION_ID'"); } /** diff --git a/server/modules/monitor/auroramon/auroramon.hh b/server/modules/monitor/auroramon/auroramon.hh index 709f78dc8..3dc70f217 100644 --- a/server/modules/monitor/auroramon/auroramon.hh +++ b/server/modules/monitor/auroramon/auroramon.hh @@ -29,7 +29,7 @@ public: static AuroraMonitor* create(const std::string& name, const std::string& module); protected: - bool has_sufficient_permissions() const; + bool has_sufficient_permissions(); void update_server_status(MXS_MONITORED_SERVER* monitored_server); private: diff --git a/server/modules/monitor/csmon/csmon.cc b/server/modules/monitor/csmon/csmon.cc index 7f42d0dba..8f4d06c85 100644 --- a/server/modules/monitor/csmon/csmon.cc +++ b/server/modules/monitor/csmon/csmon.cc @@ -86,9 +86,9 @@ CsMonitor* CsMonitor::create(const std::string& name, const std::string& module) return new CsMonitor(name, module); } -bool CsMonitor::has_sufficient_permissions() const +bool CsMonitor::has_sufficient_permissions() { - return check_monitor_permissions(m_monitor, alive_query); + return test_permissions(alive_query); } void CsMonitor::update_server_status(MXS_MONITORED_SERVER* srv) diff --git a/server/modules/monitor/csmon/csmon.hh b/server/modules/monitor/csmon/csmon.hh index 9ed36881b..59357b52a 100644 --- a/server/modules/monitor/csmon/csmon.hh +++ b/server/modules/monitor/csmon/csmon.hh @@ -25,7 +25,7 @@ public: static CsMonitor* create(const std::string& name, const std::string& module); protected: - bool has_sufficient_permissions() const; + bool has_sufficient_permissions(); void update_server_status(MXS_MONITORED_SERVER* monitored_server); private: diff --git a/server/modules/monitor/galeramon/galeramon.cc b/server/modules/monitor/galeramon/galeramon.cc index 261cf00ac..6ddef5450 100644 --- a/server/modules/monitor/galeramon/galeramon.cc +++ b/server/modules/monitor/galeramon/galeramon.cc @@ -118,9 +118,9 @@ bool GaleraMonitor::configure(const MXS_CONFIG_PARAMETER* params) return true; } -bool GaleraMonitor::has_sufficient_permissions() const +bool GaleraMonitor::has_sufficient_permissions() { - return check_monitor_permissions(m_monitor, "SHOW STATUS LIKE 'wsrep_local_state'"); + return test_permissions("SHOW STATUS LIKE 'wsrep_local_state'"); } void GaleraMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server) diff --git a/server/modules/monitor/galeramon/galeramon.hh b/server/modules/monitor/galeramon/galeramon.hh index 85738a59d..5c8e0b3b2 100644 --- a/server/modules/monitor/galeramon/galeramon.hh +++ b/server/modules/monitor/galeramon/galeramon.hh @@ -47,7 +47,7 @@ public: protected: bool configure(const MXS_CONFIG_PARAMETER* param); - bool has_sufficient_permissions() const; + bool has_sufficient_permissions(); void update_server_status(MXS_MONITORED_SERVER* monitored_server); void pre_tick(); void post_tick(); diff --git a/server/modules/monitor/grmon/grmon.cc b/server/modules/monitor/grmon/grmon.cc index 118f96bba..070072567 100644 --- a/server/modules/monitor/grmon/grmon.cc +++ b/server/modules/monitor/grmon/grmon.cc @@ -40,7 +40,7 @@ GRMon* GRMon::create(const std::string& name, const std::string& module) return new GRMon(name, module); } -bool GRMon::has_sufficient_permissions() const +bool GRMon::has_sufficient_permissions() { return true; } diff --git a/server/modules/monitor/grmon/grmon.hh b/server/modules/monitor/grmon/grmon.hh index 2f4787f3c..70c9b462a 100644 --- a/server/modules/monitor/grmon/grmon.hh +++ b/server/modules/monitor/grmon/grmon.hh @@ -29,7 +29,7 @@ public: static GRMon* create(const std::string& name, const std::string& module); protected: - bool has_sufficient_permissions() const; + bool has_sufficient_permissions(); void update_server_status(MXS_MONITORED_SERVER* monitored_server); private: diff --git a/server/modules/monitor/mmmon/mmmon.cc b/server/modules/monitor/mmmon/mmmon.cc index e320c7d2e..fc794a21a 100644 --- a/server/modules/monitor/mmmon/mmmon.cc +++ b/server/modules/monitor/mmmon/mmmon.cc @@ -69,9 +69,9 @@ bool MMMonitor::configure(const MXS_CONFIG_PARAMETER* params) return true; } -bool MMMonitor::has_sufficient_permissions() const +bool MMMonitor::has_sufficient_permissions() { - return check_monitor_permissions(m_monitor, "SHOW SLAVE STATUS"); + return test_permissions("SHOW SLAVE STATUS"); } void MMMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server) diff --git a/server/modules/monitor/mmmon/mmmon.hh b/server/modules/monitor/mmmon/mmmon.hh index db6819c7e..c2fd0850f 100644 --- a/server/modules/monitor/mmmon/mmmon.hh +++ b/server/modules/monitor/mmmon/mmmon.hh @@ -32,7 +32,7 @@ public: protected: bool configure(const MXS_CONFIG_PARAMETER* params); - bool has_sufficient_permissions() const; + bool has_sufficient_permissions(); void update_server_status(MXS_MONITORED_SERVER* monitored_server); void post_tick(); diff --git a/server/modules/monitor/ndbclustermon/ndbclustermon.cc b/server/modules/monitor/ndbclustermon/ndbclustermon.cc index 4748adc25..3437d8cb4 100644 --- a/server/modules/monitor/ndbclustermon/ndbclustermon.cc +++ b/server/modules/monitor/ndbclustermon/ndbclustermon.cc @@ -37,9 +37,9 @@ NDBCMonitor* NDBCMonitor::create(const std::string& name, const std::string& mod return new NDBCMonitor(name, module); } -bool NDBCMonitor::has_sufficient_permissions() const +bool NDBCMonitor::has_sufficient_permissions() { - return check_monitor_permissions(m_monitor, "SHOW STATUS LIKE 'Ndb_number_of_ready_data_nodes'"); + return test_permissions("SHOW STATUS LIKE 'Ndb_number_of_ready_data_nodes'"); } void NDBCMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server) diff --git a/server/modules/monitor/ndbclustermon/ndbclustermon.hh b/server/modules/monitor/ndbclustermon/ndbclustermon.hh index f216d0128..e6f26c6ab 100644 --- a/server/modules/monitor/ndbclustermon/ndbclustermon.hh +++ b/server/modules/monitor/ndbclustermon/ndbclustermon.hh @@ -29,7 +29,7 @@ public: static NDBCMonitor* create(const std::string& name, const std::string& module); protected: - bool has_sufficient_permissions() const; + bool has_sufficient_permissions(); void update_server_status(MXS_MONITORED_SERVER* monitored_server); private: