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