[Pki-devel] [PATCH] pki-ftweedal-0015-Monitor-database-for-changes-to-LDAP-profiles.patch

Fraser Tweedale ftweedal at redhat.com
Wed Apr 8 07:40:22 UTC 2015


On Mon, Apr 06, 2015 at 08:27:05PM -0500, Endi Sukma Dewata wrote:
> On 3/30/2015 2:27 AM, Fraser Tweedale wrote:
> >>1. I have not actually tested this, but suppose the LDAP server is running
> >>on a separate machine, then the network is down, and during that time a
> >>profile is removed from the database. When the network is restored, the
> >>ProfileSubsystem will reload all remaining profiles, but it will still have
> >>the profile that was deleted in its memory. I think the ProfileSubsystem
> >>should clear its memory before running the persistent search.
> >>
> >>2. In ProfileSubsystem.createProfileDN() the basedn is no longer used so it
> >>can be removed.
> >>
> >>3. If the LDAP server is stopped the debug log says it's retrying in 5
> >>seconds, but it's actually trying every second.
> >>
> >>I think #1 should be fixed, but the others are very trivial to fix too.
> >
> >New patch attached.  All issues have been addressed.
> 
> Let's say a profile is modified then deleted. These operations are
> replicated to a clone which is monitoring the changes using persistent
> search. When the clone receives the modify operation it will call
> readProfile(), which will call createProfile(), which will create an
> LDAPConfigStore for that profile. When LDAPConfigStore is created, it will
> read the profile from the database using a separate query. The problem is,
> at that point it's unclear whether the profile still exists.
> 
> The current code might ignore the error if the entry doesn't exist, but in
> general I don't think the code should make a separate query to retrieve the
> profile info because it could lead to inconsistencies. It should have used
> the LDAP entry returned by the persistent search that corresponds to the
> operation being processed.
> 
Okey doke, new patch attached.  The LDAPConfigStore no longer reads
the entry on construction, and the LDAPProfileSubsystem reads the
config from the incoming persistent search results and loads it into
the LDAPConfigStore.

Cheers!
Fraser

> -- 
> Endi S. Dewata
-------------- next part --------------
>From 5660d1653e27882f2458dfaf6764aba2f952cec0 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Mon, 22 Sep 2014 03:22:57 -0400
Subject: [PATCH] Monitor database for changes to LDAP profiles.

Use a persistent query to monitor the database for changes to LDAP
profiles, and update the contents of the ProfileSubsystem according
to the changes (Add/Modify/Delete) that occur.

The monitoring occurs within its own thread.
---
 .../com/netscape/cmscore/base/LDAPConfigStore.java |  28 +--
 .../cmscore/profile/LDAPProfileSubsystem.java      | 228 ++++++++++++++++-----
 2 files changed, 178 insertions(+), 78 deletions(-)

diff --git a/base/server/cmscore/src/com/netscape/cmscore/base/LDAPConfigStore.java b/base/server/cmscore/src/com/netscape/cmscore/base/LDAPConfigStore.java
index 788ef6bc9a26f62ed779586818525df56c555cea..a9a6536f06f36a9d41e3a01b67ec6b943a46785e 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/base/LDAPConfigStore.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/base/LDAPConfigStore.java
@@ -75,39 +75,13 @@ public class LDAPConfigStore extends PropConfigStore implements IConfigStore {
     public LDAPConfigStore(
         ILdapConnFactory dbFactory,
         String dn, LDAPAttribute[] createAttrs, String attr
-    ) throws EBaseException {
+    ) {
         super(null);  // top-level store without a name
 
         this.dbFactory = dbFactory;
         this.dn = dn;
         this.createAttrs = createAttrs;
         this.attr = attr;
-
-        LDAPConnection conn = dbFactory.getConn();
-
-        String[] readAttrs = {attr};
-        try {
-            LDAPEntry ldapEntry = conn.read(dn, readAttrs);
-
-            Enumeration<String> vals = ldapEntry.getAttribute(attr).getStringValues();
-            InputStream data = new ByteArrayInputStream(vals.nextElement().getBytes());
-            load(data);
-        } catch (LDAPException e) {
-            // if there is no such object, we will create it on commit()
-            if (e.getLDAPResultCode() != LDAPException.NO_SUCH_OBJECT) {
-                throw new EBaseException(
-                    "Error reading LDAPConfigStore '"
-                    + dn + "': " + e.toString()
-                );
-            }
-        } catch (IOException e) {
-            throw new EBaseException(
-                "Error reading LDAPConfigStore '"
-                + dn + "': " + e.toString()
-            );
-        } finally {
-            dbFactory.returnConn(conn);
-        }
     }
 
     @Override
diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
index 3572bd21d52eae0c56ac1479c65d375f08e90c2a..92aa827b1bcb348ed0b5a5df355c8ef98f265a6c 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
@@ -17,15 +17,24 @@
 // --- END COPYRIGHT BLOCK ---
 package com.netscape.cmscore.profile;
 
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.Thread;
 import java.util.Enumeration;
 import java.util.Hashtable;
 import java.util.Vector;
 
 import netscape.ldap.LDAPAttribute;
 import netscape.ldap.LDAPConnection;
+import netscape.ldap.LDAPControl;
 import netscape.ldap.LDAPEntry;
 import netscape.ldap.LDAPException;
+import netscape.ldap.LDAPSearchConstraints;
 import netscape.ldap.LDAPSearchResults;
+import netscape.ldap.controls.LDAPEntryChangeControl;
+import netscape.ldap.controls.LDAPPersistSearchControl;
+import netscape.ldap.util.DN;
 
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.base.EBaseException;
@@ -42,11 +51,14 @@ import com.netscape.cmscore.base.LDAPConfigStore;
 
 public class LDAPProfileSubsystem
         extends AbstractProfileSubsystem
-        implements IProfileSubsystem {
+        implements IProfileSubsystem, Runnable {
 
     private String dn;
     private ILdapConnFactory dbFactory;
 
+    private boolean stopped = false;
+    private Thread monitor;
+
     /**
      * Initializes this subsystem with the given configuration
      * store.
@@ -65,9 +77,6 @@ public class LDAPProfileSubsystem
         mProfiles = new Hashtable<String, IProfile>();
         mProfileClassIds = new Hashtable<String, String>();
 
-        IPluginRegistry registry = (IPluginRegistry)
-                CMS.getSubsystem(CMS.SUBSYSTEM_REGISTRY);
-
         IConfigStore cs = CMS.getConfigStore();
         IConfigStore dbCfg = cs.getSubStore("internaldb");
         dbFactory = CMS.getLdapBoundConnFactory();
@@ -85,60 +94,57 @@ public class LDAPProfileSubsystem
 
         // read profile id, implementation, and its configuration files
         String basedn = cs.getString("internaldb.basedn");
-        String dn = "ou=certificateProfiles,ou=ca," + basedn;
-        LDAPConnection conn = dbFactory.getConn();
+        dn = "ou=certificateProfiles,ou=ca," + basedn;
 
-        String[] attrs = {"cn", "classId"};
-        try {
-            LDAPSearchResults ldapProfiles = conn.search(
-                dn, LDAPConnection.SCOPE_ONE, "(objectclass=*)", attrs, false);
+        monitor = new Thread(this, "profileChangeMonitor");
+        monitor.start();
+    }
 
-            while (ldapProfiles.hasMoreElements()) {
-                String id = "<unknown>";
-                try {
-                    LDAPEntry ldapProfile = ldapProfiles.next();
+    /**
+     * Read the given LDAPEntry into the profile subsystem.
+     */
+    private void readProfile(LDAPEntry ldapProfile) {
+        IPluginRegistry registry = (IPluginRegistry)
+            CMS.getSubsystem(CMS.SUBSYSTEM_REGISTRY);
 
-                    id = (String)
-                        ldapProfile.getAttribute("cn").getStringValues().nextElement();
+        String profileId = (String)
+            ldapProfile.getAttribute("cn").getStringValues().nextElement();
 
-                    String classid = (String)
-                        ldapProfile.getAttribute("classId").getStringValues().nextElement();
+        String classId = (String)
+            ldapProfile.getAttribute("classId").getStringValues().nextElement();
 
-                    IPluginInfo info = registry.getPluginInfo("profile", classid);
-                    if (info == null) {
-                        CMS.debug("Error loading profile: No plugins for type : profile, with id " + classid);
-                    } else {
-                        CMS.debug("Start Profile Creation - " + id + " " + classid + " " + info.getClassName());
-                        createProfile(id, classid, info.getClassName());
-                        CMS.debug("Done Profile Creation - " + id);
-                    }
-                } catch (LDAPException e) {
-                    CMS.debug("Error reading profile '" + id + "'; skipping.");
-                }
-            }
-        } catch (LDAPException e) {
-            throw new EBaseException("Error reading profiles: " + e.toString());
-        } finally {
+        Enumeration<String> vals =
+            ldapProfile.getAttribute("certProfileConfig").getStringValues();
+        InputStream data = new ByteArrayInputStream(vals.nextElement().getBytes());
+
+        IPluginInfo info = registry.getPluginInfo("profile", classId);
+        if (info == null) {
+            CMS.debug("Error loading profile: No plugins for type : profile, with classId " + classId);
+        } else {
             try {
-                dbFactory.returnConn(conn);
-            } catch (Exception e) {
-                throw new EProfileException("Error releasing the ldap connection" + e.toString());
+                CMS.debug("Start Profile Creation - " + profileId + " " + classId + " " + info.getClassName());
+                createProfile(profileId, classId, info.getClassName(), data);
+                CMS.debug("Done Profile Creation - " + profileId);
+            } catch (EProfileException e) {
+                CMS.debug("Error creating profile '" + profileId + "'; skipping.");
             }
         }
+    }
 
-        Enumeration<String> ee = getProfileIds();
-
-        while (ee.hasMoreElements()) {
-            String id = ee.nextElement();
-
-            CMS.debug("Registered Confirmation - " + id);
-        }
+    public synchronized IProfile createProfile(String id, String classid, String className)
+            throws EProfileException {
+        return createProfile(id, classid, className, null);
     }
 
     /**
      * Creates a profile instance.
+     *
+     * createProfile could theoretically be called simultaneously
+     * with the same profileId from Monitor and ProfileService,
+     * so the method is synchronized.
      */
-    public IProfile createProfile(String id, String classid, String className)
+    private synchronized IProfile createProfile(
+            String id, String classid, String className, InputStream data)
             throws EProfileException {
         try {
             String[] objectClasses = {"top", "certProfile"};
@@ -150,12 +156,15 @@ public class LDAPProfileSubsystem
 
             IConfigStore subStoreConfig = new LDAPConfigStore(
                 dbFactory, createProfileDN(id), createAttrs, "certProfileConfig");
+            if (data != null)
+                subStoreConfig.load(data);
 
             CMS.debug("LDAPProfileSubsystem: initing " + className);
             IProfile profile = (IProfile) Class.forName(className).newInstance();
             profile.setId(id);
             profile.init(this, subStoreConfig);
-            mProfileIds.addElement(id);
+            if (!mProfiles.containsKey(id))
+                mProfileIds.addElement(id);
             mProfiles.put(id, profile);
             mProfileClassIds.put(id, classid);
             return profile;
@@ -187,11 +196,41 @@ public class LDAPProfileSubsystem
             }
         }
 
+        forgetProfile(id);
+    }
+
+    private synchronized void handleMODDN(DN oldDN, LDAPEntry entry) {
+        DN profilesDN = new DN(dn);
+
+        if (oldDN.isDescendantOf(profilesDN))
+            forgetProfile(oldDN.explodeDN(true)[0]);
+
+        if ((new DN(entry.getDN())).isDescendantOf(profilesDN))
+            readProfile(entry);
+    }
+
+    /**
+     * Forget a profile without deleting it from the database.
+     *
+     * This method is used when the profile change monitor receives
+     * notification that a profile was deleted.
+     */
+    private void forgetProfile(String id) {
         mProfileIds.removeElement(id);
         mProfiles.remove(id);
         mProfileClassIds.remove(id);
     }
 
+    private void forgetProfile(LDAPEntry entry) {
+        String profileId = (String)
+            entry.getAttribute("cn").getStringValues().nextElement();
+        if (profileId == null) {
+            CMS.debug("forgetProfile: error retrieving cn (profileId) from LDAPEntry");
+        } else {
+            forgetProfile(profileId);
+        }
+    }
+
     /**
      * Notifies this subsystem if owner is in running mode.
      */
@@ -205,6 +244,12 @@ public class LDAPProfileSubsystem
      * <P>
      */
     public void shutdown() {
+        stopped = true;
+        monitor = null;
+        forgetAllProfiles();
+    }
+
+    private void forgetAllProfiles() {
         mProfileIds.clear();
         mProfiles.clear();
         mProfileClassIds.clear();
@@ -217,12 +262,93 @@ public class LDAPProfileSubsystem
         if (id == null) {
             throw new EProfileException("CMS_PROFILE_ID_NOT_FOUND");
         }
-        String basedn;
-        try {
-            basedn = CMS.getConfigStore().getString("internaldb.basedn");
-        } catch (EBaseException e) {
-            throw new EProfileException("CMS_PROFILE_DELETE_UNKNOWNPROFILE");
+        return "cn=" + id + "," + dn;
+    }
+
+    public void run() {
+        int op = LDAPPersistSearchControl.ADD
+            | LDAPPersistSearchControl.MODIFY
+            | LDAPPersistSearchControl.DELETE
+            | LDAPPersistSearchControl.MODDN;
+        LDAPPersistSearchControl persistCtrl =
+            new LDAPPersistSearchControl(op, false, true, true);
+
+        LDAPConnection conn;
+
+        CMS.debug("Profile change monitor: starting.");
+
+        while (!stopped) {
+            forgetAllProfiles();
+            try {
+                conn = dbFactory.getConn();
+            } catch (ELdapException e) {
+                CMS.debug("Profile change monitor: failed to get LDAPConnection. Retrying in 1 second.");
+                try {
+                    Thread.sleep(1000);
+                } catch (InterruptedException ex) {
+                    Thread.currentThread().interrupt();
+                }
+                continue;
+            }
+            try {
+                LDAPSearchConstraints cons = conn.getSearchConstraints();
+                cons.setServerControls(persistCtrl);
+                cons.setBatchSize(1);
+                cons.setServerTimeLimit(0 /* seconds */);
+                LDAPSearchResults results = conn.search(
+                    dn, LDAPConnection.SCOPE_ONE, "(objectclass=*)",
+                    null, false, cons);
+                while (!stopped && results.hasMoreElements()) {
+                    LDAPEntry entry = results.next();
+                    LDAPEntryChangeControl changeControl = null;
+                    LDAPControl[] changeControls = results.getResponseControls();
+                    if (changeControls != null) {
+                        for (LDAPControl control : changeControls) {
+                            if (control instanceof LDAPEntryChangeControl) {
+                                changeControl = (LDAPEntryChangeControl) control;
+                                break;
+                            }
+                        }
+                    }
+                    CMS.debug("Profile change monitor: Processed change controls.");
+                    if (changeControl != null) {
+                        int changeType = changeControl.getChangeType();
+                        switch (changeType) {
+                        case LDAPPersistSearchControl.ADD:
+                            CMS.debug("Profile change monitor: ADD");
+                            readProfile(entry);
+                            break;
+                        case LDAPPersistSearchControl.DELETE:
+                            CMS.debug("Profile change monitor: DELETE");
+                            forgetProfile(entry);
+                            break;
+                        case LDAPPersistSearchControl.MODIFY:
+                            CMS.debug("Profile change monitor: MODIFY");
+                            readProfile(entry);
+                            break;
+                        case LDAPPersistSearchControl.MODDN:
+                            CMS.debug("Profile change monitor: MODDN");
+                            handleMODDN(new DN(changeControl.getPreviousDN()), entry);
+                            break;
+                        default:
+                            CMS.debug("Profile change monitor: unknown change type: " + changeType);
+                            break;
+                        }
+                    } else {
+                        CMS.debug("Profile change monitor: immediate result");
+                        readProfile(entry);
+                    }
+                }
+            } catch (LDAPException e) {
+                CMS.debug("Profile change monitor: Caught exception: " + e.toString());
+            } finally {
+                try {
+                    dbFactory.returnConn(conn);
+                } catch (Exception e) {
+                    CMS.debug("Profile change monitor: Error releasing the LDAPConnection" + e.toString());
+                }
+            }
         }
-        return "cn=" + id + ",ou=certificateProfiles,ou=ca," + basedn;
+        CMS.debug("Profile change monitor: stopping.");
     }
 }
-- 
2.1.0



More information about the Pki-devel mailing list