[Pki-devel] [PATCH] pki-ftweedal-0015-Monitor-database-for-changes-to-LDAP-profiles.patch
Fraser Tweedale
ftweedal at redhat.com
Thu Nov 20 06:32:41 UTC 2014
Thanks Endi, you picked up several subtle problems in your review.
I addressed all the issues in the new patch (attached).
Fraser
On Thu, Oct 09, 2014 at 09:47:29AM -0500, Endi Sukma Dewata wrote:
> On 10/1/2014 1:42 AM, Fraser Tweedale wrote:
> >On Mon, Sep 22, 2014 at 05:36:26PM +1000, Fraser Tweedale wrote:
> >>This is the first cut of the LDAP profile change monitoring. It
> >>depends on patches 0004..0009 and 0014
> >>(https://www.redhat.com/archives/pki-devel/2014-September/msg00052.html).
> >>
> >>To summarise the implementation: a separate thread carries out a
> >>persistent LDAP search and calls back into the ProfileSubsystem when
> >>changes occur. I haven't had much experience with persistent
> >>searches or multithreaded programming in Java, so eyeballs familiar
> >>with those areas are needed.
> >>
> >>I haven't yet tested with changes replicating between clones (a task
> >>for tomorrow) but I wanted to get the patch on list for feedback as
> >>early as possible.
> >>
> >Further to earlier email, I tested profile change replication
> >between clones and it is working well.
> >
>
> Some comments:
>
> 1. The MODDN operation can potentially happen if someone renames the profile
> in LDAP directly, and that's easier to do with LDAP-based profiles. I think
> we should handle this case too.
>
> 2. If the LDAP server is restarted the Monitor will stop working and
> subsequent changes are not detected. The Monitor should automatically
> reconnect in case the LDAP connection is interrupted.
>
> 3. Since the changesOnly is set to true, there's a possibility that changes
> that happens after initial profile loading and before the Monitor starts
> will not be detected. To avoid this problem, the changesOnly should be
> false, and the initial profile loading should be done in the Monitor as
> well.
>
> 4. The following assignment is redundant because the value is overwritten by
> the next line:
>
> String profileId = "<unknown>";
>
> 5. When receiving a MODIFY operation the Monitor calls forgetProfile(). It's
> probably redundant because readProfile() calls createProfile() which calls
> forgetProfile() too.
>
> 6. Generally it's recommended to implement Runnable instead of extending
> Thread. The Monitor can be merged into ProfileSubsystem.
>
> I think #1, #2, #3 are important to fix because there is no mechanism to
> refresh the profiles in memory other than by restarting the server, so we
> cannot miss any changes in the database.
>
>
> --
> Endi S. Dewata
-------------- next part --------------
>From 518841c8527c57f45b1b434685dd0b7276f4f366 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 10/10] 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.
---
.../netscape/cmscore/profile/ProfileSubsystem.java | 207 ++++++++++++++++-----
1 file changed, 160 insertions(+), 47 deletions(-)
diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/ProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/ProfileSubsystem.java
index ca0e09b785877da2a98fd2dd54977b8d3ebeaa24..2318fcf997801e3a729a7dab7caad3dce182486b 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/profile/ProfileSubsystem.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/profile/ProfileSubsystem.java
@@ -17,15 +17,21 @@
// --- END COPYRIGHT BLOCK ---
package com.netscape.cmscore.profile;
+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;
@@ -40,7 +46,7 @@ import com.netscape.certsrv.registry.IPluginInfo;
import com.netscape.certsrv.registry.IPluginRegistry;
import com.netscape.cmscore.base.LDAPConfigStore;
-public class ProfileSubsystem implements IProfileSubsystem {
+public class ProfileSubsystem implements IProfileSubsystem, Runnable {
private static final String PROP_CHECK_OWNER = "checkOwner";
private static final String PROP_ENABLE = "enable";
@@ -52,9 +58,13 @@ public class ProfileSubsystem implements IProfileSubsystem {
private Vector<String> mProfileIds;
private Hashtable<String, IProfile> mProfiles;
private Hashtable<String, String> mProfileClassIds;
+ private String dn;
private ILdapConnFactory dbFactory;
+ private boolean stopped = false;
+ private Thread monitor;
+
/**
* Retrieves the name of this subsystem.
*/
@@ -86,9 +96,6 @@ public class ProfileSubsystem implements IProfileSubsystem {
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();
@@ -106,60 +113,47 @@ public class ProfileSubsystem implements IProfileSubsystem {
// 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 {
+ 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());
+ 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);
- }
}
/**
* 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)
+ public synchronized IProfile createProfile(String id, String classid, String className)
throws EProfileException {
try {
String[] objectClasses = {"top", "certProfile"};
@@ -176,7 +170,8 @@ public class ProfileSubsystem implements IProfileSubsystem {
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;
@@ -211,11 +206,41 @@ public class ProfileSubsystem implements IProfileSubsystem {
}
}
+ 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.
*/
@@ -229,6 +254,8 @@ public class ProfileSubsystem implements IProfileSubsystem {
* <P>
*/
public void shutdown() {
+ stopped = true;
+ monitor = null;
mProfileIds.clear();
mProfiles.clear();
mProfileClassIds.clear();
@@ -346,6 +373,92 @@ public class ProfileSubsystem implements IProfileSubsystem {
} catch (EBaseException e) {
throw new EProfileException("CMS_PROFILE_DELETE_UNKNOWNPROFILE");
}
- return "cn=" + id + ",ou=certificateProfiles,ou=ca," + basedn;
+ 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) {
+ try {
+ conn = dbFactory.getConn();
+ } catch (ELdapException e) {
+ CMS.debug("Profile change monitor: failed to get LDAPConnection. Retrying in 5 seconds.");
+ 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());
+ }
+ }
+ }
+ CMS.debug("Profile change monitor: stopping.");
}
}
--
1.9.3
More information about the Pki-devel
mailing list