BUG #26970629 - CLUSTER CREATION FAILS IF PORT WAS NOT WRITTEN ON THE CONNECTION STRING

- The Admin API operations on a database should be done through a TCP connection
  A new validation was added to the pre-condition checks in order to verify that
  the actual session is done through TCP.

Author: rennox

modules/adminapi/mod_dba_common.cc

@@ -233,6 +233,8 @@ ReplicationGroupState check_function_preconditions(const std::string &class_name
     error = "a Classic Session is required to perform this operation";
   else if (!session->is_connected())
       error = "The session was closed. An open session is required to perform this operation";
+  else if (!session->is_tcp())
+    error = "a Classic Session through TCP/IP is required to perform this operation";
   else{
     // Retrieves the instance configuration type from the perspective of the active session
     auto instance_type = get_gr_instance_type(session->connection());

modules/base_session.cc

@@ -293,15 +293,9 @@ void ShellBaseSession::load_connection_data(const shcore::Argument_list &args) {
   // Anything found on any of the indicated sources: URI, options map and stored session
   if (2 == args.size())
     _password = args.string_at(1).c_str();
+}
 
-  // Default port will be != 0 only when applicable
-  if (_port==0) {
-      int default_port = get_default_port();
-
-    if (default_port != 0)
-      _port = default_port;
-  }
-
+void ShellBaseSession::set_uri() {
   std::string sock_port = (_port == 0) ? _sock : boost::lexical_cast<std::string>(_port);
 
   if (_schema.empty())

modules/base_session.h

@@ -90,6 +90,7 @@ class SHCORE_PUBLIC ShellBaseSession : public shcore::Cpp_object_bridge {
   struct shcore::SslInfo _ssl_info;
 
   void load_connection_data(const shcore::Argument_list &args);
+  void set_uri();
 private:
   void init();
 

modules/mod_mysql_session.cc

@@ -113,6 +113,12 @@ Value ClassicSession::connect(const Argument_list &args) {
 
     _default_schema = _schema;
 
+    // Sets the default port when required
+    if (is_tcp() && _port == 0)
+      _port = 3306;
+
+    set_uri();
+
     if (!_default_schema.empty())
       update_schema_cache(_default_schema, true);
   }

modules/mod_mysql_session.h

@@ -164,6 +164,8 @@ class SHCORE_PUBLIC ClassicSession : public ShellDevelopmentSession, public std:
 #endif
   virtual bool is_connected() const { return _conn ? true : false; }
 
+  bool is_tcp() { return _conn->is_tcp(); }
+
   virtual int get_default_port();
 
 private:

modules/mod_mysqlx_session.cc

@@ -158,6 +158,12 @@ Value BaseSession::connect(const Argument_list &args) {
 
     _connection_id = _session.get_connection_id();
 
+    // TODO(rennox): Implement is_tcp() for X protocol
+    if (_port == 0 && _sock.empty())
+      _port = 33060;
+
+    set_uri();
+
     _default_schema = _schema;
     if (!_default_schema.empty())
       update_schema_cache(_default_schema, true);

modules/mysql_connection.cc

@@ -209,7 +209,7 @@ void Connection::throw_on_connection_fail() {
 }
 
 Connection::Connection(const std::string &uri_, const char *password)
-  : _mysql(NULL) {
+  : _mysql(NULL), _tcp(false) {
   std::string protocol;
   std::string user;
   std::string pass;
@@ -258,11 +258,14 @@ Connection::Connection(const std::string &uri_, const char *password)
   if (!mysql_real_connect(_mysql, host.c_str(), user.c_str(), pass.c_str(), db.empty() ? NULL : db.c_str(), port, sock.empty() ? NULL : sock.c_str(), flags)) {
     throw_on_connection_fail();
   }
+
+  std::string connection_info(get_connection_info());
+  _tcp = (connection_info.find("via TCP/IP") != std::string::npos);
 }
 
 Connection::Connection(const std::string &host, int port, const std::string &socket, const std::string &user, const std::string &password, const std::string &schema,
   const struct shcore::SslInfo& ssl_info)
-: _mysql(NULL) {
+: _mysql(NULL), _tcp(false) {
   long flags = CLIENT_MULTI_RESULTS | CLIENT_CAN_HANDLE_EXPIRED_PASSWORDS;
 
   // No option should be silently ignored, on conflicts an error is raised
@@ -292,6 +295,9 @@ Connection::Connection(const std::string &host, int port, const std::string &soc
   if (!mysql_real_connect(_mysql, host.c_str(), user.c_str(), password.c_str(), schema.empty() ? NULL : schema.c_str(), port, socket.empty() ? NULL : socket.c_str(), flags)) {
     throw_on_connection_fail();
   }
+
+  std::string connection_info(get_connection_info());
+  _tcp = (connection_info.find("via TCP/IP") != std::string::npos);
 }
 
 bool Connection::setup_ssl(const struct shcore::SslInfo& ssl_info) {

modules/mysql_connection.h

@@ -161,13 +161,15 @@ class SHCORE_PUBLIC Connection : public std::enable_shared_from_this<Connection>
   const char* get_server_info() { _prev_result.reset(); return mysql_get_server_info(_mysql); }
   const char* get_stats() { _prev_result.reset(); return mysql_stat(_mysql); }
   const char* get_ssl_cipher() { _prev_result.reset(); return mysql_get_ssl_cipher(_mysql); }
+  bool is_tcp() { return _tcp; }
 
 private:
   bool setup_ssl(const struct shcore::SslInfo& ssl_info);
   void throw_on_connection_fail();
   std::string _uri;
   MYSQL *_mysql;
   MySQL_timer _timer;
+  bool _tcp;
 
   std::shared_ptr<MYSQL_RES> _prev_result;
 };

unittest/shell_cmdline_regressions_t.cc

@@ -16,6 +16,8 @@
 #include <string>
 #include "unittest/test_utils/command_line_test.h"
 #include "utils/utils_file.h"
+#include "utils/utils_connection.h"
+#include "modules/mysql_connection.h"
 
 namespace tests {
 
@@ -129,5 +131,50 @@ TEST_F(Command_line_test, bug24905066) {
                                   "'some_unexisting_schema'");
   }
 }
-
+TEST_F(Command_line_test, bug26970629) {
+  std::string variable;
+  std::string host;
+  std::string pwd = "--password=";
+
+  if (!_pwd.empty())
+    pwd += _pwd;
+#ifdef _WIN32
+  variable = "named_pipe";
+  host = "--host=.";
+#else
+  variable = "socket";
+  host = "--host=localhost";
+#endif
+  shcore::SslInfo info;
+  std::shared_ptr<mysqlsh::mysql::Connection> connection(
+      new mysqlsh::mysql::Connection(_host, _mysql_port_number, "", _user, _pwd,
+                                     "", info));
+
+  auto result = connection->run_sql("show variables like '" + variable + "'");
+
+  auto row = result->fetch_one();
+
+  std::string socket = "--socket=" + row->get_value_as_string(1);
+
+  connection->close();
+
+  if (socket.empty()) {
+    SCOPED_TRACE("Socket/Pipe Connections are Disabled, they must be enabled.");
+    FAIL();
+  } else {
+    std::string usr = "--user=" + _user;
+    execute({_mysqlsh, usr.c_str(), pwd.c_str(), host.c_str(), socket.c_str(),
+             "-e", "dba.getCluster()", NULL});
+    MY_EXPECT_CMD_OUTPUT_CONTAINS(
+        "Dba.getCluster: a Classic Session through TCP/IP is required to "
+        "perform this operation");
+    _output.clear();
+
+    execute({_mysqlsh, usr.c_str(), pwd.c_str(), host.c_str(), socket.c_str(),
+             "-e", "dba.createCluster('sample')", NULL});
+    MY_EXPECT_CMD_OUTPUT_CONTAINS(
+        "Dba.createCluster: a Classic Session through TCP/IP is required to "
+        "perform this operation");
+  }
+}
 }