Blob Blame Raw
From e3dea0043973faf42f7756d840bc55aa8f143eb1 Mon Sep 17 00:00:00 2001
From: William Brown <firstyear@redhat.com>
Date: Wed, 15 Nov 2017 13:44:02 +1000
Subject: [PATCH] Ticket 49298 - Correct error codes with config restore.

Bug Description:  The piece of code uses 0 as an error - not 1,
and in some cases did not even check the codes or use the
correct logic.

Fix Description:  Cleanup dse_check_file to better check the
content of files and communicate issues to the admin. Correct
slapd_bootstrap_config to correctly handle the cases of removal
and restore.

https://pagure.io/389-ds-base/issue/49298

Author: wibrown

Review by: mreynoolds & spichugi

Signed-off-by: Mark Reynolds <mreynolds@redhat.com>
(cherry picked from commit 75e55e26579955adf058e8adcba9a28779583b7b)
---
 .../suites/config/removed_config_49298_test.py     | 81 ++++++++++++++++++++++
 ldap/servers/slapd/config.c                        | 15 ++--
 ldap/servers/slapd/dse.c                           | 42 ++++++++---
 3 files changed, 119 insertions(+), 19 deletions(-)
 create mode 100644 dirsrvtests/tests/suites/config/removed_config_49298_test.py

diff --git a/dirsrvtests/tests/suites/config/removed_config_49298_test.py b/dirsrvtests/tests/suites/config/removed_config_49298_test.py
new file mode 100644
index 000000000..e65236924
--- /dev/null
+++ b/dirsrvtests/tests/suites/config/removed_config_49298_test.py
@@ -0,0 +1,81 @@
+# --- BEGIN COPYRIGHT BLOCK ---
+# Copyright (C) 2017 Red Hat, Inc.
+# All rights reserved.
+#
+# License: GPL (version 3 or any later version).
+# See LICENSE for details.
+# --- END COPYRIGHT BLOCK ---
+#
+import pytest
+import os
+import logging
+import subprocess
+
+from lib389.topologies import topology_st as topo
+
+DEBUGGING = os.getenv("DEBUGGING", default=False)
+if DEBUGGING:
+    logging.getLogger(__name__).setLevel(logging.DEBUG)
+else:
+    logging.getLogger(__name__).setLevel(logging.INFO)
+log = logging.getLogger(__name__)
+
+def test_restore_config(topo):
+    """
+    Check that if a dse.ldif and backup are removed, that the server still starts.
+
+    :id: e1c38fa7-30bc-46f2-a934-f8336f387581
+    :setup: Standalone instance
+    :steps:
+        1. Stop the instance
+        2. Delete 'dse.ldif'
+        3. Start the instance
+    :expectedresults:
+        1. Steps 1 and 2 succeed.
+        2. Server will succeed to start with restored cfg.
+    """
+    topo.standalone.stop()
+
+    dse_path = topo.standalone.get_config_dir()
+
+    log.info(dse_path)
+
+    for i in ('dse.ldif', 'dse.ldif.startOK'):
+        p = os.path.join(dse_path, i)
+        os.remove(p)
+
+    # This will pass.
+    topo.standalone.start()
+
+def test_removed_config(topo):
+    """
+    Check that if a dse.ldif and backup are removed, that the server
+    exits better than "segfault".
+
+    :id: b45272d1-c197-473e-872f-07257fcb2ec0
+    :setup: Standalone instance
+    :steps:
+        1. Stop the instance
+        2. Delete 'dse.ldif', 'dse.ldif.bak', 'dse.ldif.startOK'
+        3. Start the instance
+    :expectedresults:
+        1. Steps 1 and 2 succeed.
+        2. Server will fail to start, but will not crash.
+    """
+    topo.standalone.stop()
+
+    dse_path = topo.standalone.get_config_dir()
+
+    log.info(dse_path)
+
+    for i in ('dse.ldif', 'dse.ldif.bak', 'dse.ldif.startOK'):
+        p = os.path.join(dse_path, i)
+        os.remove(p)
+
+    # We actually can't check the log output, because it can't read dse.ldif,
+    # don't know where to write it yet! All we want is the server fail to
+    # start here, rather than infinite run + segfault.
+    with pytest.raises(subprocess.CalledProcessError):
+        topo.standalone.start()
+
+
diff --git a/ldap/servers/slapd/config.c b/ldap/servers/slapd/config.c
index afe07df84..c8d57e747 100644
--- a/ldap/servers/slapd/config.c
+++ b/ldap/servers/slapd/config.c
@@ -121,14 +121,13 @@ slapd_bootstrap_config(const char *configdir)
                       "Passed null config directory\n");
         return rc; /* Fail */
     }
-    PR_snprintf(configfile, sizeof(configfile), "%s/%s", configdir,
-                CONFIG_FILENAME);
-    PR_snprintf(tmpfile, sizeof(tmpfile), "%s/%s.tmp", configdir,
-                CONFIG_FILENAME);
-    if ((rc = dse_check_file(configfile, tmpfile)) == 0) {
-        PR_snprintf(tmpfile, sizeof(tmpfile), "%s/%s.bak", configdir,
-                    CONFIG_FILENAME);
-        rc = dse_check_file(configfile, tmpfile);
+    PR_snprintf(configfile, sizeof(configfile), "%s/%s", configdir, CONFIG_FILENAME);
+    PR_snprintf(tmpfile, sizeof(tmpfile), "%s/%s.bak", configdir, CONFIG_FILENAME);
+    rc = dse_check_file(configfile, tmpfile);
+    if (rc == 0) {
+        /* EVERYTHING IS GOING WRONG, ARRGHHHHHH */
+        slapi_log_err(SLAPI_LOG_ERR, "slapd_bootstrap_config", "No valid configurations can be accessed! You must restore %s from backup!\n", configfile);
+        return 0;
     }
 
     if ((rc = PR_GetFileInfo64(configfile, &prfinfo)) != PR_SUCCESS) {
diff --git a/ldap/servers/slapd/dse.c b/ldap/servers/slapd/dse.c
index 420248c24..653009f53 100644
--- a/ldap/servers/slapd/dse.c
+++ b/ldap/servers/slapd/dse.c
@@ -609,29 +609,49 @@ dse_check_file(char *filename, char *backupname)
 
     if (PR_GetFileInfo64(filename, &prfinfo) == PR_SUCCESS) {
         if (prfinfo.size > 0) {
-            return (1);
+            /* File exists and has content. */
+            return 1;
         } else {
+            slapi_log_err(SLAPI_LOG_INFO, "dse_check_file",
+                          "The config %s has zero length. Attempting restore ... \n", filename, rc);
             rc = PR_Delete(filename);
         }
+    } else {
+        slapi_log_err(SLAPI_LOG_INFO, "dse_check_file",
+                      "The config %s can not be accessed. Attempting restore ... (reason: %d)\n", filename, rc);
     }
 
     if (backupname) {
+
+        if (PR_GetFileInfo64(backupname, &prfinfo) != PR_SUCCESS) {
+            slapi_log_err(SLAPI_LOG_INFO, "dse_check_file",
+                          "The backup %s can not be accessed. Check it exists and permissions.\n", backupname);
+            return 0;
+        }
+
+        if (prfinfo.size <= 0) {
+            slapi_log_err(SLAPI_LOG_ERR, "dse_check_file",
+                      "The backup file %s has zero length, refusing to restore it.\n", backupname);
+            return 0;
+        }
+
         rc = PR_Rename(backupname, filename);
-    } else {
-        return (0);
-    }
+        if (rc != PR_SUCCESS) {
+            slapi_log_err(SLAPI_LOG_INFO, "dse_check_file",
+                      "The configuration file %s was NOT able to be restored from %s, error %d\n", filename, backupname, rc);
+            return 0;
+        }
 
-    if (PR_GetFileInfo64(filename, &prfinfo) == PR_SUCCESS && prfinfo.size > 0) {
         slapi_log_err(SLAPI_LOG_INFO, "dse_check_file",
-                      "The configuration file %s was restored from backup %s\n", filename, backupname);
-        return (1);
+                  "The configuration file %s was restored from backup %s\n", filename, backupname);
+        return 1;
+
     } else {
-        slapi_log_err(SLAPI_LOG_ERR, "dse_check_file",
-                      "The configuration file %s was not restored from backup %s, error %d\n",
-                      filename, backupname, rc);
-        return (0);
+        slapi_log_err(SLAPI_LOG_INFO, "dse_check_file", "No backup filename provided.\n");
+        return 0;
     }
 }
+
 static int
 dse_read_one_file(struct dse *pdse, const char *filename, Slapi_PBlock *pb, int primary_file)
 {
-- 
2.13.6