MXS-1731: Ignore empty values in persisted configs

If a MaxScale-generated configuration defines an empty value, it is
ignored with the assumption that the next modification will cause the
problem to correct itself.

maxscale-system-test/CMakeLists.txt

@@ -617,6 +617,10 @@ add_test_executable(mxs1678_relay_master.cpp mxs1678_relay_master replication LA
 # https://jira.mariadb.org/browse/MXS-1713
 add_test_executable(mxs1713_lots_of_databases.cpp mxs1713_lots_of_databases mxs1713_lots_of_databases LABELS REPL_BACKEND)
 
+# MXS-1731: Empty version_string is not detected
+# https://jira.mariadb.org/browse/MXS-1731
+add_test_executable(mxs1731_old_persisted_config.cpp mxs1731_old_persisted_config replication LABELS REPL_BACKEND)
+
 # 'namedserverfilter' test
 add_test_executable(namedserverfilter.cpp namedserverfilter namedserverfilter LABELS namedserverfilter LIGHT REPL_BACKEND)
 

maxscale-system-test/mxs1731_old_persisted_config.cpp

@@ -0,0 +1,47 @@
+/**
+ * MXS-1731: Empty version_string is not detected
+ *
+ * https://jira.mariadb.org/browse/MXS-1731
+ */
+
+#include "testconnections.h"
+#include <fstream>
+#include <iostream>
+
+using std::cout;
+using std::endl;
+
+int main(int argc, char** argv)
+{
+    TestConnections test(argc, argv);
+    const char* filename = "/tmp/RW-Split-Router.cnf";
+
+    {
+        std::ofstream cnf(filename);
+        cnf << "[RW-Split-Router]" << endl
+            << "type=service" << endl
+            << "version_string=" << endl;
+    }
+
+    test.maxscales->copy_to_node_legacy(filename, filename);
+    test.maxscales->ssh_node_f(0, true,
+                               "mkdir -p /var/lib/maxscale/maxscale.cnf.d/;"
+                               "chown maxscale:maxscale /var/lib/maxscale/maxscale.cnf.d/;"
+                               "cp %s /var/lib/maxscale/maxscale.cnf.d/RW-Split-Router.cnf", filename);
+
+    test.maxscales->restart();
+    test.check_maxscale_alive();
+
+    int rc = test.maxscales->ssh_node_f(0, true, "grep 'version_string' /var/lib/maxscale/maxscale.cnf.d/RW-Split-Router.cnf");
+    test.assert(rc == 0, "Generated configuration should have version_string defined and MaxScale should ignore it.");
+
+    test.maxscales->ssh_node_f(0, true, "maxadmin alter service RW-Split-Router enable_root_user=false");
+
+    test.maxscales->restart();
+    test.check_maxscale_alive();
+
+    rc = test.maxscales->ssh_node_f(0, true, "grep 'version_string' /var/lib/maxscale/maxscale.cnf.d/RW-Split-Router.cnf");
+    test.assert(rc != 0, "Generated configuration should not have version_string defined.");
+
+    return test.global_result;
+}

maxscale-system-test/nodes.h

@@ -132,7 +132,7 @@ public:
      * @param i Node index
      * @return exit code of the system command or 1 in case of i > N
      */
-    int copy_to_node_legacy(const char* src, const char* dest, int i);
+    int copy_to_node_legacy(const char* src, const char* dest, int i = 0);
     int copy_to_node(int i, const char* src, const char* dest);
 
     /**

server/core/config.cc

@@ -488,8 +488,19 @@ static int ini_handler(void *userdata, const char *section, const char *name, co
 
     if (is_empty_string(value))
     {
-        MXS_ERROR("Empty value given to parameter '%s'", name);
+        if (is_persisted_config)
-        return 0;
+        {
+            /**
+             * Found old-style persisted configuration. These will be automatically
+             * upgraded on the next modification so we can safely ignore it.
+             */
+            return 1;
+        }
+        else
+        {
+            MXS_ERROR("Empty value given to parameter '%s'", name);
+            return 0;
+        }
     }
 
     if (config_get_global_options()->substitute_variables)