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