From d7b49259ff2f9e0295bbfeaf128369ed33421974 Mon Sep 17 00:00:00 2001
From: James Chapman <jachapma@redhat.com>
Date: Mon, 30 Nov 2020 15:28:05 +0000
Subject: [PATCH 1/6] Issue 4418 - ldif2db - offline. Warn the user of skipped
entries
Bug Description: During an ldif2db import entries that do not
conform to various constraints will be skipped and not imported.
On completition of an import with skipped entries, the server
returns a success exit code and logs the skipped entry detail to
the error logs. The success exit code could lead the user to
believe that all entries were successfully imported.
Fix Description: If a skipped entry occurs during import, the
import will continue and a warning will be returned to the user.
CLI tools for offline import updated to handle warning code.
Test added to generate an incorrect ldif entry and perform an
import.
Fixes: #4418
Reviewed by: Firstyear, droideck (Thanks)
(cherry picked from commit a98fe54292e9b183a2163efbc7bdfe208d4abfb0)
---
.../tests/suites/import/import_test.py | 54 ++++++++++++++++++-
.../slapd/back-ldbm/db-bdb/bdb_import.c | 22 ++++++--
ldap/servers/slapd/main.c | 8 +++
ldap/servers/slapd/pblock.c | 24 +++++++++
ldap/servers/slapd/pblock_v3.h | 1 +
ldap/servers/slapd/slapi-private.h | 14 +++++
src/lib389/lib389/__init__.py | 18 +++----
src/lib389/lib389/_constants.py | 7 +++
src/lib389/lib389/cli_ctl/dbtasks.py | 8 ++-
9 files changed, 140 insertions(+), 16 deletions(-)
diff --git a/dirsrvtests/tests/suites/import/import_test.py b/dirsrvtests/tests/suites/import/import_test.py
index 3803ecf43..b47db96ed 100644
--- a/dirsrvtests/tests/suites/import/import_test.py
+++ b/dirsrvtests/tests/suites/import/import_test.py
@@ -15,7 +15,7 @@ import pytest
import time
import glob
from lib389.topologies import topology_st as topo
-from lib389._constants import DEFAULT_SUFFIX
+from lib389._constants import DEFAULT_SUFFIX, TaskWarning
from lib389.dbgen import dbgen_users
from lib389.tasks import ImportTask
from lib389.index import Indexes
@@ -139,6 +139,38 @@ def _create_bogus_ldif(topo):
return import_ldif1
+def _create_syntax_err_ldif(topo):
+ """
+ Create an incorrect ldif entry that violates syntax check
+ """
+ ldif_dir = topo.standalone.get_ldif_dir()
+ line1 = """dn: dc=example,dc=com
+objectClass: top
+objectClass: domain
+dc: example
+dn: ou=groups,dc=example,dc=com
+objectClass: top
+objectClass: organizationalUnit
+ou: groups
+dn: uid=JHunt,ou=groups,dc=example,dc=com
+objectClass: top
+objectClass: person
+objectClass: organizationalPerson
+objectClass: inetOrgPerson
+objectclass: inetUser
+cn: James Hunt
+sn: Hunt
+uid: JHunt
+givenName:
+"""
+ with open(f'{ldif_dir}/syntax_err.ldif', 'w') as out:
+ out.write(f'{line1}')
+ os.chmod(out.name, 0o777)
+ out.close()
+ import_ldif1 = ldif_dir + '/syntax_err.ldif'
+ return import_ldif1
+
+
def test_import_with_index(topo, _import_clean):
"""
Add an index, then import via cn=tasks
@@ -214,6 +246,26 @@ def test_ldif2db_allows_entries_without_a_parent_to_be_imported(topo, _import_cl
topo.standalone.start()
+def test_ldif2db_syntax_check(topo):
+ """ldif2db should return a warning when a skipped entry has occured.
+ :id: 85e75670-42c5-4062-9edc-7f117c97a06f
+ :setup:
+ 1. Standalone Instance
+ 2. Ldif entry that violates syntax check rule (empty givenname)
+ :steps:
+ 1. Create an ldif file which violates the syntax checking rule
+ 2. Stop the server and import ldif file with ldif2db
+ :expected results:
+ 1. ldif2db import returns a warning to signify skipped entries
+ """
+ import_ldif1 = _create_syntax_err_ldif(topo)
+ # Import the offending LDIF data - offline
+ topo.standalone.stop()
+ ret = topo.standalone.ldif2db('userRoot', None, None, None, import_ldif1)
+ assert ret == TaskWarning.WARN_SKIPPED_IMPORT_ENTRY
+ topo.standalone.start()
+
+
def test_issue_a_warning_if_the_cache_size_is_smaller(topo, _import_clean):
"""Report during startup if nsslapd-cachememsize is too small
diff --git a/ldap/servers/slapd/back-ldbm/db-bdb/bdb_import.c b/ldap/servers/slapd/back-ldbm/db-bdb/bdb_import.c
index e7da0517f..1e4830e99 100644
--- a/ldap/servers/slapd/back-ldbm/db-bdb/bdb_import.c
+++ b/ldap/servers/slapd/back-ldbm/db-bdb/bdb_import.c
@@ -2563,7 +2563,7 @@ error:
slapi_task_dec_refcount(job->task);
}
import_all_done(job, ret);
- ret = 1;
+ ret |= WARN_UPGARDE_DN_FORMAT_ALL;
} else if (NEED_DN_NORM == ret) {
import_log_notice(job, SLAPI_LOG_NOTICE, "bdb_import_main",
"%s complete. %s needs upgradednformat.",
@@ -2572,7 +2572,7 @@ error:
slapi_task_dec_refcount(job->task);
}
import_all_done(job, ret);
- ret = 2;
+ ret |= WARN_UPGRADE_DN_FORMAT;
} else if (NEED_DN_NORM_SP == ret) {
import_log_notice(job, SLAPI_LOG_NOTICE, "bdb_import_main",
"%s complete. %s needs upgradednformat spaces.",
@@ -2581,7 +2581,7 @@ error:
slapi_task_dec_refcount(job->task);
}
import_all_done(job, ret);
- ret = 3;
+ ret |= WARN_UPGRADE_DN_FORMAT_SPACE;
} else {
ret = -1;
if (job->task != NULL) {
@@ -2600,6 +2600,11 @@ error:
import_all_done(job, ret);
}
+ /* set task warning if there are no errors */
+ if((!ret) && (job->skipped)) {
+ ret |= WARN_SKIPPED_IMPORT_ENTRY;
+ }
+
/* This instance isn't busy anymore */
instance_set_not_busy(job->inst);
@@ -2637,6 +2642,7 @@ bdb_back_ldif2db(Slapi_PBlock *pb)
int total_files, i;
int up_flags = 0;
PRThread *thread = NULL;
+ int ret = 0;
slapi_pblock_get(pb, SLAPI_BACKEND, &be);
if (be == NULL) {
@@ -2764,7 +2770,15 @@ bdb_back_ldif2db(Slapi_PBlock *pb)
}
/* old style -- do it all synchronously (THIS IS GOING AWAY SOON) */
- return import_main_offline((void *)job);
+ ret = import_main_offline((void *)job);
+
+ /* no error just warning, reset ret */
+ if(ret &= WARN_SKIPPED_IMPORT_ENTRY) {
+ slapi_pblock_set_task_warning(pb, WARN_SKIPPED_IMPORT_ENTRY);
+ ret = 0;
+ }
+
+ return ret;
}
struct _import_merge_thang
diff --git a/ldap/servers/slapd/main.c b/ldap/servers/slapd/main.c
index 694375b22..104f6826c 100644
--- a/ldap/servers/slapd/main.c
+++ b/ldap/servers/slapd/main.c
@@ -2069,6 +2069,14 @@ slapd_exemode_ldif2db(struct main_config *mcfg)
plugin->plg_name);
return_value = -1;
}
+
+ /* check for task warnings */
+ if(!return_value) {
+ if((return_value = slapi_pblock_get_task_warning(pb))) {
+ slapi_log_err(SLAPI_LOG_INFO, "slapd_exemode_ldif2db","returning task warning: %d\n", return_value);
+ }
+ }
+
slapi_pblock_destroy(pb);
charray_free(instances);
charray_free(mcfg->cmd_line_instance_names);
diff --git a/ldap/servers/slapd/pblock.c b/ldap/servers/slapd/pblock.c
index 454ea9cc3..1ad9d0399 100644
--- a/ldap/servers/slapd/pblock.c
+++ b/ldap/servers/slapd/pblock.c
@@ -28,12 +28,14 @@
#define SLAPI_LDIF_DUMP_REPLICA 2003
#define SLAPI_PWDPOLICY 2004
#define SLAPI_PW_ENTRY 2005
+#define SLAPI_TASK_WARNING 2006
/* Used for checking assertions about pblocks in some cases. */
#define SLAPI_HINT 9999
static PRLock *pblock_analytics_lock = NULL;
+
static PLHashNumber
hash_int_func(const void *key)
{
@@ -4315,6 +4317,28 @@ slapi_pblock_set_ldif_dump_replica(Slapi_PBlock *pb, int32_t dump_replica)
pb->pb_task->ldif_dump_replica = dump_replica;
}
+int32_t
+slapi_pblock_get_task_warning(Slapi_PBlock *pb)
+{
+#ifdef PBLOCK_ANALYTICS
+ pblock_analytics_record(pb, SLAPI_TASK_WARNING);
+#endif
+ if (pb->pb_task != NULL) {
+ return pb->pb_task->task_warning;
+ }
+ return 0;
+}
+
+void
+slapi_pblock_set_task_warning(Slapi_PBlock *pb, task_warning warning)
+{
+#ifdef PBLOCK_ANALYTICS
+ pblock_analytics_record(pb, SLAPI_TASK_WARNING);
+#endif
+ _pblock_assert_pb_task(pb);
+ pb->pb_task->task_warning = warning;
+}
+
void *
slapi_pblock_get_vattr_context(Slapi_PBlock *pb)
{
diff --git a/ldap/servers/slapd/pblock_v3.h b/ldap/servers/slapd/pblock_v3.h
index 90498c0b0..b35d78565 100644
--- a/ldap/servers/slapd/pblock_v3.h
+++ b/ldap/servers/slapd/pblock_v3.h
@@ -67,6 +67,7 @@ typedef struct _slapi_pblock_task
int ldif2db_noattrindexes;
int ldif_printkey;
int task_flags;
+ int32_t task_warning;
int import_state;
int server_running; /* indicate that server is running */
diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h
index c98c1947c..31cb33472 100644
--- a/ldap/servers/slapd/slapi-private.h
+++ b/ldap/servers/slapd/slapi-private.h
@@ -1465,6 +1465,20 @@ void slapi_pblock_set_operation_notes(Slapi_PBlock *pb, uint32_t opnotes);
void slapi_pblock_set_flag_operation_notes(Slapi_PBlock *pb, uint32_t opflag);
void slapi_pblock_set_result_text_if_empty(Slapi_PBlock *pb, char *text);
+/* task warnings */
+typedef enum task_warning_t{
+ WARN_UPGARDE_DN_FORMAT_ALL = (1 << 0),
+ WARN_UPGRADE_DN_FORMAT = (1 << 1),
+ WARN_UPGRADE_DN_FORMAT_SPACE = (1 << 2),
+ WARN_SKIPPED_IMPORT_ENTRY = (1 << 3)
+} task_warning;
+
+int32_t slapi_pblock_get_task_warning(Slapi_PBlock *pb);
+void slapi_pblock_set_task_warning(Slapi_PBlock *pb, task_warning warn);
+
+
+int slapi_exists_or_add_internal(Slapi_DN *dn, const char *filter, const char *entry, const char *modifier_name);
+
#ifdef __cplusplus
}
#endif
diff --git a/src/lib389/lib389/__init__.py b/src/lib389/lib389/__init__.py
index 4e6a1905a..5b36a79e1 100644
--- a/src/lib389/lib389/__init__.py
+++ b/src/lib389/lib389/__init__.py
@@ -2683,7 +2683,7 @@ class DirSrv(SimpleLDAPObject, object):
# server is stopped)
#
def ldif2db(self, bename, suffixes, excludeSuffixes, encrypt,
- import_file):
+ import_file, import_cl):
"""
@param bename - The backend name of the database to import
@param suffixes - List/tuple of suffixes to import
@@ -2731,14 +2731,14 @@ class DirSrv(SimpleLDAPObject, object):
try:
result = subprocess.check_output(cmd, encoding='utf-8')
except subprocess.CalledProcessError as e:
- self.log.debug("Command: %s failed with the return code %s and the error %s",
- format_cmd_list(cmd), e.returncode, e.output)
- return False
-
- self.log.debug("ldif2db output: BEGIN")
- for line in result.split("\n"):
- self.log.debug(line)
- self.log.debug("ldif2db output: END")
+ if e.returncode == TaskWarning.WARN_SKIPPED_IMPORT_ENTRY:
+ self.log.debug("Command: %s skipped import entry warning %s",
+ format_cmd_list(cmd), e.returncode)
+ return e.returncode
+ else:
+ self.log.debug("Command: %s failed with the return code %s and the error %s",
+ format_cmd_list(cmd), e.returncode, e.output)
+ return False
return True
diff --git a/src/lib389/lib389/_constants.py b/src/lib389/lib389/_constants.py
index e28c602a3..38ba04565 100644
--- a/src/lib389/lib389/_constants.py
+++ b/src/lib389/lib389/_constants.py
@@ -162,6 +162,13 @@ DB2BAK = 'db2bak'
DB2INDEX = 'db2index'
DBSCAN = 'dbscan'
+# Task warnings
+class TaskWarning(IntEnum):
+ WARN_UPGARDE_DN_FORMAT_ALL = (1 << 0)
+ WARN_UPGRADE_DN_FORMAT = (1 << 1)
+ WARN_UPGRADE_DN_FORMAT_SPACE = (1 << 2)
+ WARN_SKIPPED_IMPORT_ENTRY = (1 << 3)
+
RDN_REPLICA = "cn=replica"
RETROCL_SUFFIX = "cn=changelog"
diff --git a/src/lib389/lib389/cli_ctl/dbtasks.py b/src/lib389/lib389/cli_ctl/dbtasks.py
index 590a1ea0e..02830239c 100644
--- a/src/lib389/lib389/cli_ctl/dbtasks.py
+++ b/src/lib389/lib389/cli_ctl/dbtasks.py
@@ -7,6 +7,7 @@
# See LICENSE for details.
# --- END COPYRIGHT BLOCK ---
+from lib389._constants import TaskWarning
def dbtasks_db2index(inst, log, args):
if not inst.db2index(bename=args.backend):
@@ -44,10 +45,13 @@ def dbtasks_db2ldif(inst, log, args):
def dbtasks_ldif2db(inst, log, args):
- if not inst.ldif2db(bename=args.backend, encrypt=args.encrypted, import_file=args.ldif,
- suffixes=None, excludeSuffixes=None):
+ ret = inst.ldif2db(bename=args.backend, encrypt=args.encrypted, import_file=args.ldif,
+ suffixes=None, excludeSuffixes=None, import_cl=False)
+ if not ret:
log.fatal("ldif2db failed")
return False
+ elif ret == TaskWarning.WARN_SKIPPED_IMPORT_ENTRY:
+ log.warn("ldif2db successful with skipped entries")
else:
log.info("ldif2db successful")
--
2.26.2