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