Blob Blame History Raw
From 70d1336481bd1a36d8b0bdef43a9364c7db58c26 Mon Sep 17 00:00:00 2001
From: Ludwig Krispenz <lkrispen@redhat.com>
Date: Wed, 20 Feb 2019 10:11:15 +0100
Subject: [PATCH] Ticket 50232 - export creates not importable ldif file

Bug: If the RUV entry hasa  smaller entryid than the suffix entry it will be
	exported before the suffix. If that ldif is used for import the RUV entry
	is skipped and a new one generated with a different database generation

Fix: Before exporting the RUV check that the suffix is alread exported, if not
	make the RUV entry pending and write it after all othere entries

Reviewed by: tbordaz, wbrown. Thanks
---
 dirsrvtests/tests/tickets/ticket50232_test.py | 163 ++++++++++++++++++
 ldap/servers/slapd/back-ldbm/ldif2ldbm.c      |  58 ++++++-
 2 files changed, 219 insertions(+), 2 deletions(-)
 create mode 100644 dirsrvtests/tests/tickets/ticket50232_test.py

diff --git a/dirsrvtests/tests/tickets/ticket50232_test.py b/dirsrvtests/tests/tickets/ticket50232_test.py
new file mode 100644
index 000000000..133ed0dfe
--- /dev/null
+++ b/dirsrvtests/tests/tickets/ticket50232_test.py
@@ -0,0 +1,163 @@
+# --- BEGIN COPYRIGHT BLOCK ---
+# Copyright (C) 2016 Red Hat, Inc.
+# All rights reserved.
+#
+# License: GPL (version 3 or any later version).
+# See LICENSE for details.
+# --- END COPYRIGHT BLOCK ---
+#
+import logging
+
+import pytest
+# from lib389.tasks import *
+# from lib389.utils import *
+from lib389.topologies import topology_st
+from lib389.replica import ReplicationManager,Replicas
+
+from lib389._constants import DEFAULT_SUFFIX, BACKEND_NAME
+
+from lib389.idm.user import UserAccounts
+from lib389.idm.organization import Organization
+from lib389.idm.organizationalunit import OrganizationalUnit
+
+log = logging.getLogger(__name__)
+
+NORMAL_SUFFIX = 'o=normal'
+NORMAL_BACKEND_NAME = 'normal'
+REVERSE_SUFFIX = 'o=reverse'
+REVERSE_BACKEND_NAME = 'reverse'
+
+def _enable_replica(instance, suffix):
+
+    repl = ReplicationManager(DEFAULT_SUFFIX)
+    repl._ensure_changelog(instance)
+    replicas = Replicas(instance)
+    replicas.create(properties={
+        'cn': 'replica',
+        'nsDS5ReplicaRoot': suffix,
+        'nsDS5ReplicaId': '1',
+        'nsDS5Flags': '1',
+        'nsDS5ReplicaType': '3'
+        })
+
+def _populate_suffix(instance, suffixname):
+
+    o = Organization(instance, 'o={}'.format(suffixname))
+    o.create(properties={
+        'o': suffixname,
+        'description': 'test'
+    })
+    ou = OrganizationalUnit(instance, 'ou=people,o={}'.format(suffixname))
+    ou.create(properties={
+        'ou': 'people'
+    })
+
+def _get_replica_generation(instance, suffix):
+
+    replicas = Replicas(instance)
+    replica = replicas.get(suffix)
+    ruv = replica.get_ruv()
+    return ruv._data_generation
+
+def _test_export_import(instance, suffix, backend):
+
+    before_generation = _get_replica_generation(instance, suffix)
+
+    instance.stop()
+    instance.db2ldif(
+        bename=backend,
+        suffixes=[suffix],
+        excludeSuffixes=[],
+        encrypt=False,
+        repl_data=True,
+        outputfile="/tmp/output_file",
+    )
+    instance.ldif2db(
+        bename=None,
+        excludeSuffixes=None,
+        encrypt=False,
+        suffixes=[suffix],
+        import_file="/tmp/output_file",
+    )
+    instance.start()
+    after_generation = _get_replica_generation(instance, suffix)
+
+    assert (before_generation == after_generation)
+
+def test_ticket50232_normal(topology_st):
+    """
+    The fix for ticket 50232
+
+
+    The test sequence is:
+    - create suffix
+    - add suffix entry and some child entries
+    - "normally" done after populating suffix: enable replication
+    - get RUV and database generation
+    - export -r
+    - import
+    - get RUV and database generation
+    - assert database generation has not changed
+    """
+
+    log.info('Testing Ticket 50232 - export creates not imprtable ldif file, normal creation order')
+
+    topology_st.standalone.backend.create(NORMAL_SUFFIX, {BACKEND_NAME: NORMAL_BACKEND_NAME})
+    topology_st.standalone.mappingtree.create(NORMAL_SUFFIX, bename=NORMAL_BACKEND_NAME, parent=None)
+
+    _populate_suffix(topology_st.standalone, NORMAL_BACKEND_NAME)
+
+    repl = ReplicationManager(DEFAULT_SUFFIX)
+    repl._ensure_changelog(topology_st.standalone)
+    replicas = Replicas(topology_st.standalone)
+    replicas.create(properties={
+        'cn': 'replica',
+        'nsDS5ReplicaRoot': NORMAL_SUFFIX,
+        'nsDS5ReplicaId': '1',
+        'nsDS5Flags': '1',
+        'nsDS5ReplicaType': '3'
+        })
+
+    _test_export_import(topology_st.standalone, NORMAL_SUFFIX, NORMAL_BACKEND_NAME)
+
+def test_ticket50232_reverse(topology_st):
+    """
+    The fix for ticket 50232
+
+
+    The test sequence is:
+    - create suffix
+    - enable replication before suffix enztry is added
+    - add suffix entry and some child entries
+    - get RUV and database generation
+    - export -r
+    - import
+    - get RUV and database generation
+    - assert database generation has not changed
+    """
+
+    log.info('Testing Ticket 50232 - export creates not imprtable ldif file, normal creation order')
+
+    #
+    # Setup Replication
+    #
+    log.info('Setting up replication...')
+    repl = ReplicationManager(DEFAULT_SUFFIX)
+    # repl.create_first_master(topology_st.standalone)
+    #
+    # enable dynamic plugins, memberof and retro cl plugin
+    #
+    topology_st.standalone.backend.create(REVERSE_SUFFIX, {BACKEND_NAME: REVERSE_BACKEND_NAME})
+    topology_st.standalone.mappingtree.create(REVERSE_SUFFIX, bename=REVERSE_BACKEND_NAME, parent=None)
+
+    _enable_replica(topology_st.standalone, REVERSE_SUFFIX)
+
+    _populate_suffix(topology_st.standalone, REVERSE_BACKEND_NAME)
+
+    _test_export_import(topology_st.standalone, REVERSE_SUFFIX, REVERSE_BACKEND_NAME)
+
+if __name__ == '__main__':
+    # Run isolated
+    # -s for DEBUG mode
+    CURRENT_FILE = os.path.realpath(__file__)
+    pytest.main("-s %s" % CURRENT_FILE)
diff --git a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
index 11c020af0..49fe7cd5d 100644
--- a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
+++ b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
@@ -1103,6 +1103,7 @@ bail:
  * (reunified at last)
  */
 #define LDBM2LDIF_BUSY (-2)
+#define RUVRDN SLAPI_ATTR_UNIQUEID "=" RUV_STORAGE_ENTRY_UNIQUEID
 int
 ldbm_back_ldbm2ldif(Slapi_PBlock *pb)
 {
@@ -1111,6 +1112,7 @@ ldbm_back_ldbm2ldif(Slapi_PBlock *pb)
     DB *db = NULL;
     DBC *dbc = NULL;
     struct backentry *ep;
+    struct backentry *pending_ruv = NULL;
     DBT key = {0};
     DBT data = {0};
     char *fname = NULL;
@@ -1146,6 +1148,8 @@ ldbm_back_ldbm2ldif(Slapi_PBlock *pb)
     static int load_dse = 1; /* We'd like to load dse just once. */
     int server_running;
     export_args eargs = {0};
+    int32_t suffix_written = 0;
+    int32_t skip_ruv = 0;
 
     slapi_log_err(SLAPI_LOG_TRACE, "ldbm_back_ldbm2ldif", "=>\n");
 
@@ -1463,8 +1467,25 @@ ldbm_back_ldbm2ldif(Slapi_PBlock *pb)
                 }
             }
 
-            if (0 != return_value)
+            if (DB_NOTFOUND == return_value) {
+                /* reached the end of the database,
+                 * check if ruv is pending and write it
+                 */
+                if (pending_ruv) {
+                    eargs.ep = pending_ruv;
+                    eargs.idindex = idindex;
+                    eargs.cnt = &cnt;
+                    eargs.lastcnt = &lastcnt;
+                    rc = export_one_entry(li, inst, &eargs);
+                    backentry_free(&pending_ruv);
+                }
+                break;
+            }
+
+            if (0 != return_value) {
+                /* error reading database */
                 break;
+            }
 
             /* back to internal format */
             temp_id = id_stored_to_internal((char *)key.data);
@@ -1501,7 +1522,30 @@ ldbm_back_ldbm2ldif(Slapi_PBlock *pb)
                 rc = get_value_from_string((const char *)data.dptr,
                                            LDBM_PARENTID_STR, &pid_str);
                 if (rc) {
-                    rc = 0; /* assume this is a suffix */
+                    /* this could be a suffix or the RUV entry.
+                     * If it is the ruv and the suffix is not written
+                     * keep the ruv and export as last entry.
+                     *
+                     * The reason for this is that if the RUV entry is in the
+                     * ldif before the suffix entry then at an attempt to import
+                     * that ldif the RUV entry would be skipped because the parent
+                     * does not exist. Later a new RUV would be generated with
+                     * a different database generation and replication is broken
+                     */
+                    if (suffix_written) {
+                        /* this must be the RUV, just continue and write it */
+                        rc = 0;
+                    } else if (0 == strcasecmp(rdn, RUVRDN)) {
+                        /* this is the RUV and the suffix is not yet written
+                         * make it pending and continue with next entry
+                         */
+                        skip_ruv = 1;
+                        rc = 0;
+                    } else {
+                        /* this has to be the suffix */
+                        suffix_written = 1;
+                        rc = 0;
+                    }
                 } else {
                     pid = (ID)strtol(pid_str, (char **)NULL, 10);
                     slapi_ch_free_string(&pid_str);
@@ -1614,6 +1658,16 @@ ldbm_back_ldbm2ldif(Slapi_PBlock *pb)
             continue;
         }
 
+        if (skip_ruv) {
+            /* now we keep a copy of the ruv entry
+             * and continue with the next entry
+             */
+            pending_ruv = ep;
+            ep = NULL;
+            skip_ruv = 0;
+            continue;
+        }
+
         eargs.ep = ep;
         eargs.idindex = idindex;
         eargs.cnt = &cnt;
-- 
2.17.2