andykimpe / rpms / 389-ds-base

Forked from rpms/389-ds-base 6 months ago
Clone

Blame SOURCES/0020-Issue-4418-ldif2db-offline.-Warn-the-user-of-skipped.patch

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