From 43f8a317bcd9040874b27cad905347a9e6bc8a6f Mon Sep 17 00:00:00 2001
From: James Chapman <jachapma@redhat.com>
Date: Wed, 9 Dec 2020 22:42:59 +0000
Subject: [PATCH 4/6] Issue 4419 - Warn users of skipped entries during ldif2db
online import (#4476)
Bug Description: During an online 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 responds with a success message
and logs the skipped entry detail to the error logs.
The success messgae 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 message will be displayed.
The schema is extended with a nsTaskWarning attribute
which is used to capture and retrieve any task
warnings.
CLI tools for online import updated.
Test added to generate an incorrect ldif entry and perform an
online import.
Fixes: https://github.com/389ds/389-ds-base/issues/4419
Reviewed by: tbordaz, mreynolds389, droideck, Firstyear (Thanks)
---
.../tests/suites/import/import_test.py | 39 +++++++++++++++++--
ldap/schema/02common.ldif | 3 +-
.../back-ldbm/db-bdb/bdb_import_threads.c | 5 +++
ldap/servers/slapd/slap.h | 1 +
ldap/servers/slapd/slapi-plugin.h | 11 ++++++
ldap/servers/slapd/slapi-private.h | 8 ----
ldap/servers/slapd/task.c | 29 +++++++++++++-
src/lib389/lib389/cli_conf/backend.py | 6 ++-
src/lib389/lib389/tasks.py | 23 +++++++++--
9 files changed, 108 insertions(+), 17 deletions(-)
diff --git a/dirsrvtests/tests/suites/import/import_test.py b/dirsrvtests/tests/suites/import/import_test.py
index b47db96ed..77c915026 100644
--- a/dirsrvtests/tests/suites/import/import_test.py
+++ b/dirsrvtests/tests/suites/import/import_test.py
@@ -65,6 +65,9 @@ def _import_clean(request, topo):
import_ldif = ldif_dir + '/basic_import.ldif'
if os.path.exists(import_ldif):
os.remove(import_ldif)
+ syntax_err_ldif = ldif_dir + '/syntax_err.dif'
+ if os.path.exists(syntax_err_ldif):
+ os.remove(syntax_err_ldif)
request.addfinalizer(finofaci)
@@ -141,17 +144,19 @@ def _create_bogus_ldif(topo):
def _create_syntax_err_ldif(topo):
"""
- Create an incorrect ldif entry that violates syntax check
+ Create an ldif file, which contains an 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
@@ -201,6 +206,34 @@ def test_import_with_index(topo, _import_clean):
assert f'{place}/userRoot/roomNumber.db' in glob.glob(f'{place}/userRoot/*.db', recursive=True)
+def test_online_import_with_warning(topo, _import_clean):
+ """
+ Import an ldif file with syntax errors, verify skipped entry warning code
+
+ :id: 5bf75c47-a283-430e-a65c-3c5fd8dbadb8
+ :setup: Standalone Instance
+ :steps:
+ 1. Create standalone Instance
+ 2. Create an ldif file with an entry that violates syntax check (empty givenname)
+ 3. Online import of troublesome ldif file
+ :expected results:
+ 1. Successful import with skipped entry warning
+ """
+ topo.standalone.restart()
+
+ import_task = ImportTask(topo.standalone)
+ import_ldif1 = _create_syntax_err_ldif(topo)
+
+ # Importing the offending ldif file - online
+ import_task.import_suffix_from_ldif(ldiffile=import_ldif1, suffix=DEFAULT_SUFFIX)
+
+ # There is just a single entry in this ldif
+ import_task.wait(5)
+
+ # Check for the task nsTaskWarning attr, make sure its set to skipped entry code
+ assert import_task.present('nstaskwarning')
+ assert TaskWarning.WARN_SKIPPED_IMPORT_ENTRY == import_task.get_task_warn()
+
def test_crash_on_ldif2db(topo, _import_clean):
"""
Delete the cn=monitor entry for an LDBM backend instance. Doing this will
@@ -246,7 +279,7 @@ def test_ldif2db_allows_entries_without_a_parent_to_be_imported(topo, _import_cl
topo.standalone.start()
-def test_ldif2db_syntax_check(topo):
+def test_ldif2db_syntax_check(topo, _import_clean):
"""ldif2db should return a warning when a skipped entry has occured.
:id: 85e75670-42c5-4062-9edc-7f117c97a06f
:setup:
@@ -261,7 +294,7 @@ def test_ldif2db_syntax_check(topo):
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)
+ ret = topo.standalone.ldif2db('userRoot', None, None, None, import_ldif1, None)
assert ret == TaskWarning.WARN_SKIPPED_IMPORT_ENTRY
topo.standalone.start()
diff --git a/ldap/schema/02common.ldif b/ldap/schema/02common.ldif
index c6dc074db..821640d03 100644
--- a/ldap/schema/02common.ldif
+++ b/ldap/schema/02common.ldif
@@ -145,6 +145,7 @@ attributeTypes: ( 2.16.840.1.113730.3.1.2356 NAME 'nsTaskExitCode' DESC 'Slapi T
attributeTypes: ( 2.16.840.1.113730.3.1.2357 NAME 'nsTaskCurrentItem' DESC 'Slapi Task item' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
attributeTypes: ( 2.16.840.1.113730.3.1.2358 NAME 'nsTaskTotalItems' DESC 'Slapi Task total items' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
attributeTypes: ( 2.16.840.1.113730.3.1.2359 NAME 'nsTaskCreated' DESC 'Slapi Task creation date' SYNTAX 1.3.6.1.4.1.1466.115.121.1.24 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
+attributeTypes: ( 2.16.840.1.113730.3.1.2375 NAME 'nsTaskWarning' DESC 'Slapi Task warning code' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN '389 Directory Server' )
#
# objectclasses:
#
@@ -177,5 +178,5 @@ objectClasses: ( 2.16.840.1.113730.3.2.503 NAME 'nsDSWindowsReplicationAgreement
objectClasses: ( 2.16.840.1.113730.3.2.128 NAME 'costemplate' DESC 'Netscape defined objectclass' SUP top MAY ( cn $ cospriority ) X-ORIGIN 'Netscape Directory Server' )
objectClasses: ( 2.16.840.1.113730.3.2.304 NAME 'nsView' DESC 'Netscape defined objectclass' SUP top AUXILIARY MAY ( nsViewFilter $ description ) X-ORIGIN 'Netscape Directory Server' )
objectClasses: ( 2.16.840.1.113730.3.2.316 NAME 'nsAttributeEncryption' DESC 'Netscape defined objectclass' SUP top MUST ( cn $ nsEncryptionAlgorithm ) X-ORIGIN 'Netscape Directory Server' )
-objectClasses: ( 2.16.840.1.113730.3.2.335 NAME 'nsSlapiTask' DESC 'Slapi_Task objectclass' SUP top MUST ( cn ) MAY ( ttl $ nsTaskLog $ nsTaskStatus $ nsTaskExitCode $ nsTaskCurrentItem $ nsTaskTotalItems $ nsTaskCreated ) X-ORIGIN '389 Directory Server' )
+objectClasses: ( 2.16.840.1.113730.3.2.335 NAME 'nsSlapiTask' DESC 'Slapi_Task objectclass' SUP top MUST ( cn ) MAY ( ttl $ nsTaskLog $ nsTaskStatus $ nsTaskExitCode $ nsTaskCurrentItem $ nsTaskTotalItems $ nsTaskCreated $ nsTaskWarning ) X-ORIGIN '389 Directory Server' )
diff --git a/ldap/servers/slapd/back-ldbm/db-bdb/bdb_import_threads.c b/ldap/servers/slapd/back-ldbm/db-bdb/bdb_import_threads.c
index 310893884..5c7d9c8f7 100644
--- a/ldap/servers/slapd/back-ldbm/db-bdb/bdb_import_threads.c
+++ b/ldap/servers/slapd/back-ldbm/db-bdb/bdb_import_threads.c
@@ -747,6 +747,11 @@ import_producer(void *param)
}
}
+ /* capture skipped entry warnings for this task */
+ if((job) && (job->skipped)) {
+ slapi_task_set_warning(job->task, WARN_SKIPPED_IMPORT_ENTRY);
+ }
+
slapi_value_free(&(job->usn_value));
import_free_ldif(&c);
info->state = FINISHED;
diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h
index 53c9161d1..be4d38739 100644
--- a/ldap/servers/slapd/slap.h
+++ b/ldap/servers/slapd/slap.h
@@ -1753,6 +1753,7 @@ typedef struct slapi_task
int task_progress; /* number between 0 and task_work */
int task_work; /* "units" of work to be done */
int task_flags; /* (see above) */
+ task_warning task_warn; /* task warning */
char *task_status; /* transient status info */
char *task_log; /* appended warnings, etc */
char task_date[SLAPI_TIMESTAMP_BUFSIZE]; /* Date/time when task was created */
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 96313ef2c..ddb11bc7c 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -6638,6 +6638,15 @@ int slapi_config_remove_callback(int operation, int flags, const char *base, int
/* task flags (set by the task-control code) */
#define SLAPI_TASK_DESTROYING 0x01 /* queued event for destruction */
+/* 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;
+
+
int slapi_task_register_handler(const char *name, dseCallbackFn func);
int slapi_plugin_task_register_handler(const char *name, dseCallbackFn func, Slapi_PBlock *plugin_pb);
int slapi_plugin_task_unregister_handler(const char *name, dseCallbackFn func);
@@ -6654,6 +6663,8 @@ int slapi_task_get_refcount(Slapi_Task *task);
void slapi_task_set_destructor_fn(Slapi_Task *task, TaskCallbackFn func);
void slapi_task_set_cancel_fn(Slapi_Task *task, TaskCallbackFn func);
void slapi_task_status_changed(Slapi_Task *task);
+void slapi_task_set_warning(Slapi_Task *task, task_warning warn);
+int slapi_task_get_warning(Slapi_Task *task);
void slapi_task_log_status(Slapi_Task *task, char *format, ...)
#ifdef __GNUC__
__attribute__((format(printf, 2, 3)));
diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h
index d5abe8ac1..b956ebe63 100644
--- a/ldap/servers/slapd/slapi-private.h
+++ b/ldap/servers/slapd/slapi-private.h
@@ -1465,14 +1465,6 @@ 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);
diff --git a/ldap/servers/slapd/task.c b/ldap/servers/slapd/task.c
index 936c64920..806077a16 100644
--- a/ldap/servers/slapd/task.c
+++ b/ldap/servers/slapd/task.c
@@ -46,6 +46,7 @@ static uint64_t shutting_down = 0;
#define TASK_PROGRESS_NAME "nsTaskCurrentItem"
#define TASK_WORK_NAME "nsTaskTotalItems"
#define TASK_DATE_NAME "nsTaskCreated"
+#define TASK_WARNING_NAME "nsTaskWarning"
#define DEFAULT_TTL "3600" /* seconds */
#define TASK_SYSCONFIG_FILE_ATTR "sysconfigfile" /* sysconfig reload task file attr */
@@ -332,7 +333,7 @@ slapi_task_status_changed(Slapi_Task *task)
LDAPMod modlist[20];
LDAPMod *mod[20];
int cur = 0, i;
- char s1[20], s2[20], s3[20];
+ char s1[20], s2[20], s3[20], s4[20];
if (shutting_down) {
/* don't care about task status updates anymore */
@@ -346,9 +347,11 @@ slapi_task_status_changed(Slapi_Task *task)
sprintf(s1, "%d", task->task_exitcode);
sprintf(s2, "%d", task->task_progress);
sprintf(s3, "%d", task->task_work);
+ sprintf(s4, "%d", task->task_warn);
NEXTMOD(TASK_PROGRESS_NAME, s2);
NEXTMOD(TASK_WORK_NAME, s3);
NEXTMOD(TASK_DATE_NAME, task->task_date);
+ NEXTMOD(TASK_WARNING_NAME, s4);
/* only add the exit code when the job is done */
if ((task->task_state == SLAPI_TASK_FINISHED) ||
(task->task_state == SLAPI_TASK_CANCELLED)) {
@@ -452,6 +455,30 @@ slapi_task_get_refcount(Slapi_Task *task)
return 0; /* return value not currently used */
}
+/*
+ * Return task warning
+ */
+int
+slapi_task_get_warning(Slapi_Task *task)
+{
+ if (task) {
+ return task->task_warn;
+ }
+
+ return 0; /* return value not currently used */
+}
+
+/*
+ * Set task warning
+ */
+void
+slapi_task_set_warning(Slapi_Task *task, task_warning warn)
+{
+ if (task) {
+ return task->task_warn |= warn;
+ }
+}
+
int
slapi_plugin_task_unregister_handler(const char *name, dseCallbackFn func)
{
diff --git a/src/lib389/lib389/cli_conf/backend.py b/src/lib389/lib389/cli_conf/backend.py
index d7a6e670c..6bfbcb036 100644
--- a/src/lib389/lib389/cli_conf/backend.py
+++ b/src/lib389/lib389/cli_conf/backend.py
@@ -243,9 +243,13 @@ def backend_import(inst, basedn, log, args):
exclude_suffixes=args.exclude_suffixes)
task.wait(timeout=None)
result = task.get_exit_code()
+ warning = task.get_task_warn()
if task.is_complete() and result == 0:
- log.info("The import task has finished successfully")
+ if warning is None or (warning == 0):
+ log.info("The import task has finished successfully")
+ else:
+ log.info("The import task has finished successfully, with warning code {}, check the logs for more detail".format(warning))
else:
raise ValueError("Import task failed\n-------------------------\n{}".format(ensure_str(task.get_task_log())))
diff --git a/src/lib389/lib389/tasks.py b/src/lib389/lib389/tasks.py
index dc7bb9206..bf20d1e61 100644
--- a/src/lib389/lib389/tasks.py
+++ b/src/lib389/lib389/tasks.py
@@ -38,6 +38,7 @@ class Task(DSLdapObject):
self._protected = False
self._exit_code = None
self._task_log = ""
+ self._task_warn = None
def status(self):
"""Return the decoded status of the task
@@ -49,6 +50,7 @@ class Task(DSLdapObject):
self._exit_code = self.get_attr_val_utf8("nsTaskExitCode")
self._task_log = self.get_attr_val_utf8("nsTaskLog")
+ self._task_warn = self.get_attr_val_utf8("nsTaskWarning")
if not self.exists():
self._log.debug("complete: task has self cleaned ...")
# The task cleaned it self up.
@@ -77,6 +79,15 @@ class Task(DSLdapObject):
return None
return None
+ def get_task_warn(self):
+ """Return task's warning code if task is complete, else None."""
+ if self.is_complete():
+ try:
+ return int(self._task_warn)
+ except TypeError:
+ return None
+ return None
+
def wait(self, timeout=120):
"""Wait until task is complete."""
@@ -390,14 +401,17 @@ class Tasks(object):
running, true if done - if true, second is the exit code - if dowait
is True, this function will block until the task is complete'''
attrlist = ['nsTaskLog', 'nsTaskStatus', 'nsTaskExitCode',
- 'nsTaskCurrentItem', 'nsTaskTotalItems']
+ 'nsTaskCurrentItem', 'nsTaskTotalItems', 'nsTaskWarning']
done = False
exitCode = 0
+ warningCode = 0
dn = entry.dn
while not done:
entry = self.conn.getEntry(dn, attrlist=attrlist)
self.log.debug("task entry %r", entry)
+ if entry.nsTaskWarning:
+ warningCode = int(entry.nsTaskWarning)
if entry.nsTaskExitCode:
exitCode = int(entry.nsTaskExitCode)
done = True
@@ -405,7 +419,7 @@ class Tasks(object):
time.sleep(1)
else:
break
- return (done, exitCode)
+ return (done, exitCode, warningCode)
def importLDIF(self, suffix=None, benamebase=None, input_file=None,
args=None):
@@ -461,8 +475,9 @@ class Tasks(object):
self.conn.add_s(entry)
exitCode = 0
+ warningCode = 0
if args and args.get(TASK_WAIT, False):
- (done, exitCode) = self.conn.tasks.checkTask(entry, True)
+ (done, exitCode, warningCode) = self.conn.tasks.checkTask(entry, True)
if exitCode:
self.log.error("Error: import task %s for file %s exited with %d",
@@ -470,6 +485,8 @@ class Tasks(object):
else:
self.log.info("Import task %s for file %s completed successfully",
cn, input_file)
+ if warningCode:
+ self.log.info("with warning code %d", warningCode)
self.dn = dn
self.entry = entry
return exitCode
--
2.26.2